* Re: [PATCH v6 4/4] mmc: sdio support external scatter gather list
From: Ulf Hansson @ 2017-12-19 12:39 UTC (permalink / raw)
To: Xinming Hu
Cc: Linux MMC, Kalle Valo, Arend van Spriel, Franky Lin,
Hante Meuleman, Chi-Hsien Lin, Wright Feng, Zhiyuan Yang,
Tim Song, Cathy Luo, James Cao, Ganapathi Bhat, Xu Wang, Bob Tan,
Amitkumar Karwar
In-Reply-To: <1513685211-640-4-git-send-email-huxm@marvell.com>
On 19 December 2017 at 13:06, Xinming Hu <huxm@marvell.com> wrote:
> Currently sdio device drivers need to prepare a big continuous
> physical memory for large data transfer. mmc stack will construct
> scatter/gather buffer list to make use of ADMA2.
>
> According to the sdio host controller specification version 4.00
> chapter 1.13.2, sdio device drivers have the ability to construct
> scatter/gather DMA buffer list. In this way, they do not need to allocate
> big memory.
>
> This patch refactors current sdio command 53 wrapper mmc_io_rw_extended,
> so that it can accept external scatter/gather buffer list from its caller,
> such as the sdio device driver. This patch is very useful on some embedded
> systems where large memory allocation fails sometimes. It gives 10 to 15%
> better throughput performance compared to a case in which small single
> data transfers are done.
>
> Signed-off-by: Xinming Hu <huxm@marvell.com>
> Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
> ---
> v4: same as v3/v2/v1
> v5: rebased on latest codebase
> v6: same as v5
> ---
> drivers/mmc/core/sdio_io.c | 22 ++++++++++++++++--
> drivers/mmc/core/sdio_ops.c | 53 +++++++++++++++++++++++++++++++++++++++----
> drivers/mmc/core/sdio_ops.h | 3 ++-
> include/linux/mmc/sdio_func.h | 6 +++++
> 4 files changed, 76 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/mmc/core/sdio_io.c b/drivers/mmc/core/sdio_io.c
> index 4806521..6046511 100644
> --- a/drivers/mmc/core/sdio_io.c
> +++ b/drivers/mmc/core/sdio_io.c
> @@ -327,7 +327,7 @@ static int sdio_io_rw_ext_helper(struct sdio_func *func, int write,
> size = blocks * func->cur_blksize;
>
> ret = mmc_io_rw_extended(func->card, write,
> - func->num, addr, incr_addr, buf,
> + func->num, addr, incr_addr, false, buf,
> blocks, func->cur_blksize);
> if (ret)
> return ret;
> @@ -345,7 +345,7 @@ static int sdio_io_rw_ext_helper(struct sdio_func *func, int write,
>
> /* Indicate byte mode by setting "blocks" = 0 */
> ret = mmc_io_rw_extended(func->card, write, func->num, addr,
> - incr_addr, buf, 0, size);
> + incr_addr, false, buf, 0, size);
> if (ret)
> return ret;
>
> @@ -513,6 +513,24 @@ int sdio_writesb(struct sdio_func *func, unsigned int addr, void *src,
> }
> EXPORT_SYMBOL_GPL(sdio_writesb);
>
> +int sdio_readsb_sg_enh(struct sdio_func *func, unsigned int addr,
Could you please rename this to sdio_readsb_sg() instead?
> + struct sg_table *dst)
I think what Christoph suggested and I agree with, is to use a "struct
scatterlist *sg" instead of "struct sg_table *dst" as the
in-parameter.
The similar applies for the write API, of course.
> +{
> + return mmc_io_rw_extended(func->card, 0, func->num,
> + addr, 0, true,
> + dst, 0, func->cur_blksize);
> +}
> +EXPORT_SYMBOL_GPL(sdio_readsb_sg_enh);
> +
> +int sdio_writesb_sg_enh(struct sdio_func *func, unsigned int addr,
Could you please rename this to sdio_writesb_sg() instead?
> + struct sg_table *src)
> +{
> + return mmc_io_rw_extended(func->card, 1, func->num,
> + addr, 0, true,
> + src, 0, func->cur_blksize);
> +}
> +EXPORT_SYMBOL_GPL(sdio_writesb_sg_enh);
> +
> /**
> * sdio_readw - read a 16 bit integer from a SDIO function
> * @func: SDIO function to access
> diff --git a/drivers/mmc/core/sdio_ops.c b/drivers/mmc/core/sdio_ops.c
> index abaaba3..2310a2a 100644
> --- a/drivers/mmc/core/sdio_ops.c
> +++ b/drivers/mmc/core/sdio_ops.c
> @@ -115,16 +115,39 @@ int mmc_io_rw_direct(struct mmc_card *card, int write, unsigned fn,
> return mmc_io_rw_direct_host(card->host, write, fn, addr, in, out);
> }
>
> +/**
> + * mmc_io_rw_extended - wrapper for sdio command 53 read/write operation
> + * @card: destination device for read/write
> + * @write: zero indcate read, non-zero indicate write.
> + * @fn: SDIO function to access
> + * @addr: address of (single byte) FIFO
> + * @incr_addr: set to 1 indicate read or write multiple bytes
> + of data to/from an IO register address that
> + increment by 1 after each operation.
> + * @sg_enhance: if set true, the caller of this function will
> + prepare scatter gather dma buffer list.
> + * @buf: buffer that contains the data to write. if sg_enhance
> + is set, it point to SG dma buffer list.
> + * @blocks: number of blocks of data transfer.
> + if set zero, indicate byte mode
> + if set non-zero, indicate block mode
> + if sg_enhance is set, this parameter will not be used.
> + * @blksz: block size for block mode data transfer.
> + *
> + */
The descriptive function header isn't required here, because this is
an internal function for the mmc core.
However, if you really want this, I suggest you make it a separate patch.
> int mmc_io_rw_extended(struct mmc_card *card, int write, unsigned fn,
> - unsigned addr, int incr_addr, u8 *buf, unsigned blocks, unsigned blksz)
> + unsigned int addr, int incr_addr, bool sg_enhance,
Instead of letting this take a new bool variable, let's instead
provide it with a "struct scatterlist *sg". In cases when it isn't
used, just provide NULL.
Depending on the end result, we may even consider to make a separate
function dealing with the scatterlist case, perhaps via sharing some
common functionality from mmc_io_rw_extended() instead.
> + void *buf, unsigned int blocks, unsigned int blksz)
> {
> struct mmc_request mrq = {};
> struct mmc_command cmd = {};
> struct mmc_data data = {};
> struct scatterlist sg, *sg_ptr;
> struct sg_table sgtable;
> + struct sg_table *sgtable_external;
> unsigned int nents, left_size, i;
> unsigned int seg_size = card->host->max_seg_size;
> + unsigned int sg_blocks = 0;
>
> WARN_ON(blksz == 0);
>
> @@ -140,16 +163,35 @@ int mmc_io_rw_extended(struct mmc_card *card, int write, unsigned fn,
> cmd.arg |= fn << 28;
> cmd.arg |= incr_addr ? 0x04000000 : 0x00000000;
> cmd.arg |= addr << 9;
> + cmd.flags = MMC_RSP_SPI_R5 | MMC_RSP_R5 | MMC_CMD_ADTC;
> +
> + data.flags = write ? MMC_DATA_WRITE : MMC_DATA_READ;
> + data.blksz = blksz;
> +
> + if (sg_enhance) {
> + sgtable_external = buf;
> +
> + for_each_sg(sgtable_external->sgl, sg_ptr,
> + sgtable_external->nents, i) {
> + if (sg_ptr->length > card->host->max_seg_size)
> + return -EINVAL;
> + sg_blocks += DIV_ROUND_UP(sg_ptr->length, blksz);
This looks wrong. For each iteration the number of sg_blocks will be
increased, with the round up method.
This may lead to that the data.blocks could get a in-correct value
(bigger) when assigned below, right?
> + }
> +
> + cmd.arg |= 0x08000000 | sg_blocks;
This isn't going to work for byte mode transfers. Seems like that
should be possible too when using a pre-allocated scatterlist, right!?
> + data.blocks = sg_blocks;
> + data.sg = sgtable_external->sgl;
> + data.sg_len = sgtable_external->nents;
> + goto mmc_data_req;
> + }
> +
> if (blocks == 0)
> cmd.arg |= (blksz == 512) ? 0 : blksz; /* byte mode */
> else
> cmd.arg |= 0x08000000 | blocks; /* block mode */
> - cmd.flags = MMC_RSP_SPI_R5 | MMC_RSP_R5 | MMC_CMD_ADTC;
>
> - data.blksz = blksz;
> /* Code in host drivers/fwk assumes that "blocks" always is >=1 */
> data.blocks = blocks ? blocks : 1;
> - data.flags = write ? MMC_DATA_WRITE : MMC_DATA_READ;
>
> left_size = data.blksz * data.blocks;
> nents = DIV_ROUND_UP(left_size, seg_size);
> @@ -172,11 +214,12 @@ int mmc_io_rw_extended(struct mmc_card *card, int write, unsigned fn,
> sg_init_one(&sg, buf, left_size);
> }
>
> +mmc_data_req:
> mmc_set_data_timeout(&data, card);
>
> mmc_wait_for_req(card->host, &mrq);
>
> - if (nents > 1)
> + if (!sg_enhance && nents > 1)
> sg_free_table(&sgtable);
>
> if (cmd.error)
> diff --git a/drivers/mmc/core/sdio_ops.h b/drivers/mmc/core/sdio_ops.h
> index 96945ca..2d75100 100644
> --- a/drivers/mmc/core/sdio_ops.h
> +++ b/drivers/mmc/core/sdio_ops.h
> @@ -23,7 +23,8 @@
> int mmc_io_rw_direct(struct mmc_card *card, int write, unsigned fn,
> unsigned addr, u8 in, u8* out);
> int mmc_io_rw_extended(struct mmc_card *card, int write, unsigned fn,
> - unsigned addr, int incr_addr, u8 *buf, unsigned blocks, unsigned blksz);
> + unsigned int addr, int incr_addr, bool sg_enhance,
> + void *buf, unsigned int blocks, unsigned int blksz);
> int sdio_reset(struct mmc_host *host);
> unsigned int mmc_align_data_size(struct mmc_card *card, unsigned int sz);
> void sdio_irq_work(struct work_struct *work);
> diff --git a/include/linux/mmc/sdio_func.h b/include/linux/mmc/sdio_func.h
> index 72d4de4..d36ba47 100644
> --- a/include/linux/mmc/sdio_func.h
> +++ b/include/linux/mmc/sdio_func.h
> @@ -14,6 +14,7 @@
>
> #include <linux/device.h>
> #include <linux/mod_devicetable.h>
> +#include <linux/scatterlist.h>
>
> #include <linux/mmc/pm.h>
>
> @@ -136,6 +137,11 @@ extern int sdio_memcpy_fromio(struct sdio_func *func, void *dst,
> extern int sdio_readsb(struct sdio_func *func, void *dst,
> unsigned int addr, int count);
>
> +int sdio_readsb_sg_enh(struct sdio_func *func, unsigned int addr,
> + struct sg_table *dst);
> +int sdio_writesb_sg_enh(struct sdio_func *func, unsigned int addr,
> + struct sg_table *src);
> +
> extern void sdio_writeb(struct sdio_func *func, u8 b,
> unsigned int addr, int *err_ret);
> extern void sdio_writew(struct sdio_func *func, u16 b,
> --
> 1.9.1
>
Kind regards
Uffe
^ permalink raw reply
* Re: [PATCH V5] mmc:host:sdhci-pci:Addition of Arasan PCI Controller with integrated phy.
From: Adrian Hunter @ 2017-12-19 12:18 UTC (permalink / raw)
To: Atul Garg, linux-mmc, kishon, rk, nm, nsekhar, ulf.hansson; +Cc: linux-kernel
In-Reply-To: <1513098128-4776-1-git-send-email-agarg@arasan.com>
On 12/12/17 19:02, Atul Garg wrote:
> The Arasan Controller is based on a FPGA platform and has integrated phy
> with specific registers used during initialization and
> management of different modes. The phy and the controller are integrated
> and registers are very specific to Arasan.
>
> Arasan being an IP provider, licenses these IPs to various companies for
> integration of IP in custom SOCs. The custom SOCs define own register
> map depending on how bits are tied inside the SOC for phy registers,
> depending on SOC memory plan and hence will require own platform drivers.
>
> If more details on phy regsiters are required, an interface document is
> hosted at https: //arasandotcom/NF/eMMC5.1 PHY Programming in Linux.pdf.
>
> Signed-off-by: Atul Garg <agarg@arasan.com>
Sorry for the slow reply. Looks good. Some very minor comments below.
Also I had trouble applying it so it would be good to have a new version
based on the latest mmc next.
> ---
> V5 - Separated arasan_phy_poll function into arasan_phy_addr_poll and
> arasan_phy_sts_poll Tabspace corrected. Checked return values of poll functions.
> Removed static declaration of sdhci_pci_enable_dma and defined in
> sdhci-pci.h as suggested by Adrian Hunter <adrian.hunter@intel.com>.
> V4 - Created arasan_phy_poll function to have common timeout call.
> .Restructured arasan_set_phy to arasan_select_phy_clock and
> .arasan_phy_set to have single set of registers to be programmed for different modes.
> .Applied code style suggestions from Adrian Hunter <adrian.hunter@intel.com>.
> V3 - Removed sdhci-pci-arasan.h. Code and interface document mentioned
> above are made relevant..Applied code style suggestions from
> Sekhar Nori <nsekhar@ti.com> and .Adrian Hunter <adrian.hunter@intel.com>.
> V2 - Removed code from sdhci-pci-core.c and created sdhci-pci-arasan.c and
> .sdhci-pci-arasan.h.
> V1 - Initial Patch coded in sdhci-pci-core.c.
>
> drivers/mmc/host/Makefile | 2 +-
> drivers/mmc/host/sdhci-pci-arasan.c | 341 ++++++++++++++++++++++++++++++++++++
> drivers/mmc/host/sdhci-pci-core.c | 4 +-
> drivers/mmc/host/sdhci-pci.h | 7 +-
> 4 files changed, 350 insertions(+), 4 deletions(-)
> create mode 100644 drivers/mmc/host/sdhci-pci-arasan.c
>
> diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
> index ab61a3e..6781b81 100644
> --- a/drivers/mmc/host/Makefile
> +++ b/drivers/mmc/host/Makefile
> @@ -10,7 +10,7 @@ obj-$(CONFIG_MMC_MXC) += mxcmmc.o
> obj-$(CONFIG_MMC_MXS) += mxs-mmc.o
> obj-$(CONFIG_MMC_SDHCI) += sdhci.o
> obj-$(CONFIG_MMC_SDHCI_PCI) += sdhci-pci.o
> -sdhci-pci-y += sdhci-pci-core.o sdhci-pci-o2micro.o
> +sdhci-pci-y += sdhci-pci-core.o sdhci-pci-o2micro.o sdhci-pci-arasan.o
> obj-$(subst m,y,$(CONFIG_MMC_SDHCI_PCI)) += sdhci-pci-data.o
> obj-$(CONFIG_MMC_SDHCI_ACPI) += sdhci-acpi.o
> obj-$(CONFIG_MMC_SDHCI_PXAV3) += sdhci-pxav3.o
> diff --git a/drivers/mmc/host/sdhci-pci-arasan.c b/drivers/mmc/host/sdhci-pci-arasan.c
> new file mode 100644
> index 0000000..9df64a1
> --- /dev/null
> +++ b/drivers/mmc/host/sdhci-pci-arasan.c
> @@ -0,0 +1,341 @@
> +/*
> + * Copyright (C) 2017 Arasan Chip Systems Inc.
> + *
> + * Author: Atul Garg <agarg@arasan.com>
> + *
> + * This software is licensed under the terms of the GNU General Public
> + * License version 2, as published by the Free Software Foundation, and
> + * may be copied, distributed, and modified under those terms.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/pci.h>
> +#include <linux/delay.h>
> +
> +#include "sdhci.h"
> +#include "sdhci-pci.h"
> +
> +/* Extra registers for Arasan SD Host Controller for eMMC5.1 PHY */
> +#define PHY_ADDR_REG 0x300
> +#define PHY_DAT_REG 0x304
> +
> +#define PHY_WRITE BIT(8)
> +#define PHY_BUSY BIT(9)
> +#define DATA_MASK 0xFF
> +
> +/* eMMC5.1 PHY Specific Registers */
> +#define DLL_STS 0x00
> +#define IPAD_CTRL1 0x01
> +#define IPAD_CTRL2 0x02
> +#define IPAD_STS 0x03
> +#define IOREN_CTRL1 0x06
> +#define IOREN_CTRL2 0x07
> +#define IOPU_CTRL1 0x08
> +#define IOPU_CTRL2 0x09
> +#define ITAP_DELAY 0x0C
> +#define OTAP_DELAY 0x0D
> +#define STRB_SEL 0x0E
> +#define CLKBUF_SEL 0x0F
> +#define MODE_CTRL 0x11
> +#define DLL_TRIM 0x12
> +#define CMD_CTRL 0x20
> +#define DATA_CTRL 0x21
> +#define STRB_CTRL 0x22
> +#define CLK_CTRL 0x23
> +#define PHY_CTRL 0x24
> +
> +#define DLL_EN BIT(3)
> +#define RTRIM_EN BIT(1)
> +#define PDB_EN BIT(1)
> +#define RETB_EN BIT(6)
> +#define ODEN_CMD BIT(1)
> +#define ODEN_DAT 0xFF
> +#define REN_STRB BIT(0)
> +#define REN_CMD BIT(1)
> +#define REN_DAT 0xFF
> +#define PU_CMD BIT(1)
> +#define PU_DAT 0xFF
> +#define ITAPDLY_EN BIT(0)
> +#define OTAPDLY_EN BIT(0)
> +#define OD_REL_CMD BIT(1)
> +#define OD_REL_DAT 0xFF
> +#define DLLTRM_ICP 0x8
> +#define PDB_CMD BIT(0)
> +#define PDB_DAT 0xFF
> +#define PDB_STRB BIT(0)
> +#define PDB_CLK BIT(0)
> +#define CALDONE_MASK 0x10
> +#define DLL_RDY_MASK 0x10
> +#define MAX_CLK_BUF 0x7
> +
> +/* Mode Controls */
> +#define ENHSTRB_MODE BIT(0)
> +#define HS400_MODE BIT(1)
> +#define LEGACY_MODE BIT(2)
> +#define DDR50_MODE BIT(3)
> +
> +/*
> + * Controller has no specific bits for HS/HS200.
> + * Used BIT(4), BIT(5) for software programming.
> + */
> +#define HS200_MODE BIT(4)
> +#define HS_MODE BIT(5)
The defines still don't line up. They should line up in the source code
when the tab width is set to 8.
> +
> +#define OTAPDLY(x) ((x << 1) | OTAPDLY_EN)
> +#define ITAPDLY(x) ((x << 1) | ITAPDLY_EN)
> +#define FREQSEL(x) ((x << 5) | DLL_EN)
> +#define IOPAD(x, y) ((x) | (y << 2))
Some tools get upset if they see macros where the parameters are not
enclosed in parentheses i.e. should be:
#define OTAPDLY(x) (((x) << 1) | OTAPDLY_EN)
#define ITAPDLY(x) (((x) << 1) | ITAPDLY_EN)
#define FREQSEL(x) (((x) << 5) | DLL_EN)
#define IOPAD(x, y) ((x) | ((y) << 2))
> +
> +/* Arasan private data */
> +struct arasan_host {
> + u32 chg_clk;
> +};
> +
> +static int arasan_phy_addr_poll(struct sdhci_host *host, u32 offset, u32 mask)
> +{
> + ktime_t timeout = ktime_add_us(ktime_get(), 100);
> + bool failed;
> + u8 val = 0;
> +
> + while (1) {
> + failed = ktime_after(ktime_get(), timeout);
> + val = sdhci_readw(host, PHY_ADDR_REG);
> + if (!(val & mask))
> + return 0;
> + if (failed)
> + return -EBUSY;
> + }
> +}
> +
> +static int arasan_phy_write(struct sdhci_host *host, u8 data, u8 offset)
> +{
> + int ret;
> +
> + sdhci_writew(host, data, PHY_DAT_REG);
> + sdhci_writew(host, (PHY_WRITE | offset), PHY_ADDR_REG);
> + ret = arasan_phy_addr_poll(host, PHY_ADDR_REG, PHY_BUSY);
> + return ret;
Should be just:
return arasan_phy_addr_poll(host, PHY_ADDR_REG, PHY_BUSY);
Then ret is not needed.
> +}
> +
> +static int arasan_phy_read(struct sdhci_host *host, u8 offset, u8 *data)
> +{
> + int ret;
> +
> + sdhci_writew(host, 0, PHY_DAT_REG);
> + sdhci_writew(host, offset, PHY_ADDR_REG);
> + ret = arasan_phy_addr_poll(host, PHY_ADDR_REG, PHY_BUSY);
> +
> + /* Masking valid data bits */
> + *data = sdhci_readw(host, PHY_DAT_REG) & DATA_MASK;
> + return ret;
> +}
> +
> +static int arasan_phy_sts_poll(struct sdhci_host *host, u32 offset, u32 mask)
> +{
> + int ret;
> + ktime_t timeout = ktime_add_us(ktime_get(), 100);
> + bool failed;
> + u8 val = 0;
> +
> + while (1) {
> + failed = ktime_after(ktime_get(), timeout);
> + ret = arasan_phy_read(host, offset, &val);
> + if (ret)
> + return -EBUSY;
> + else if (val & mask)
> + return 0;
> + if (failed)
> + return -EBUSY;
> + }
> +}
> +
> +/* Initialize the Arasan PHY */
> +static int arasan_phy_init(struct sdhci_host *host)
> +{
> + int ret;
> + u8 val;
> +
> + /* Program IOPADs and wait for calibration to be done */
> + if (arasan_phy_read(host, IPAD_CTRL1, &val) ||
> + arasan_phy_write(host, val | RETB_EN | PDB_EN, IPAD_CTRL1) ||
> + arasan_phy_read(host, IPAD_CTRL2, &val) ||
> + arasan_phy_write(host, val | RTRIM_EN, IPAD_CTRL2))
> + return -EBUSY;
> + ret = arasan_phy_sts_poll(host, IPAD_STS, CALDONE_MASK);
> + if (ret)
> + return -EBUSY;
> +
> + /* Program CMD/Data lines */
> + if (arasan_phy_read(host, IOREN_CTRL1, &val) ||
> + arasan_phy_write(host, val | REN_CMD | REN_STRB, IOREN_CTRL1) ||
> + arasan_phy_read(host, IOPU_CTRL1, &val) ||
> + arasan_phy_write(host, val | PU_CMD, IOPU_CTRL1) ||
> + arasan_phy_read(host, CMD_CTRL, &val) ||
> + arasan_phy_write(host, val | PDB_CMD, CMD_CTRL) ||
> + arasan_phy_read(host, IOREN_CTRL2, &val) ||
> + arasan_phy_write(host, val | REN_DAT, IOREN_CTRL2) ||
> + arasan_phy_read(host, IOPU_CTRL2, &val) ||
> + arasan_phy_write(host, val | PU_DAT, IOPU_CTRL2) ||
> + arasan_phy_read(host, DATA_CTRL, &val) ||
> + arasan_phy_write(host, val | PDB_DAT, DATA_CTRL) ||
> + arasan_phy_read(host, STRB_CTRL, &val) ||
> + arasan_phy_write(host, val | PDB_STRB, STRB_CTRL) ||
> + arasan_phy_read(host, CLK_CTRL, &val) ||
> + arasan_phy_write(host, val | PDB_CLK, CLK_CTRL) ||
> + arasan_phy_read(host, CLKBUF_SEL, &val) ||
> + arasan_phy_write(host, val | MAX_CLK_BUF, CLKBUF_SEL) ||
> + arasan_phy_write(host, LEGACY_MODE, MODE_CTRL))
> + return -EBUSY;
> + return 0;
> +}
> +
> +/* Set Arasan PHY for different modes */
> +static int arasan_phy_set(struct sdhci_host *host, u8 mode, u8 otap,
> + u8 drv_type, u8 itap, u8 trim, u8 clk)
> +{
> + u8 val;
> + int ret;
> +
> + if (mode == HS_MODE || mode == HS200_MODE)
> + ret = arasan_phy_write(host, 0x0, MODE_CTRL);
> + else
> + ret = arasan_phy_write(host, mode, MODE_CTRL);
> + if (ret)
> + return ret;
> + if (mode == HS400_MODE || mode == HS200_MODE) {
> + ret = arasan_phy_read(host, IPAD_CTRL1, &val);
> + if (ret)
> + return ret;
> + ret = arasan_phy_write(host, IOPAD(val, drv_type), IPAD_CTRL1);
> + if (ret)
> + return ret;
> + }
> + if (mode == LEGACY_MODE) {
> + ret = arasan_phy_write(host, 0x0, OTAP_DELAY);
> + if (ret)
> + return ret;
> + ret = arasan_phy_write(host, 0x0, ITAP_DELAY);
> + } else {
> + ret = arasan_phy_write(host, OTAPDLY(otap), OTAP_DELAY);
> + if (ret)
> + return ret;
> + if (mode != HS200_MODE)
> + ret = arasan_phy_write(host, ITAPDLY(itap), ITAP_DELAY);
> + else
> + ret = arasan_phy_write(host, 0x0, ITAP_DELAY);
> + }
> + if (ret)
> + return ret;
> + if (mode != LEGACY_MODE) {
> + ret = arasan_phy_write(host, trim, DLL_TRIM);
> + if (ret)
> + return ret;
> + }
> + ret = arasan_phy_write(host, 0, DLL_STS);
> + if (ret)
> + return ret;
> + if (mode != LEGACY_MODE) {
> + ret = arasan_phy_write(host, FREQSEL(clk), DLL_STS);
> + if (ret)
> + return ret;
> + ret = arasan_phy_sts_poll(host, DLL_STS, DLL_RDY_MASK);
> + if (ret)
> + return -EBUSY;
> + }
> + return 0;
> +}
> +
> +static int arasan_select_phy_clock(struct sdhci_host *host)
> +{
> + struct sdhci_pci_slot *slot = sdhci_priv(host);
> + struct arasan_host *arasan_host = sdhci_pci_priv(slot);
> + u8 clk;
> +
> + if (arasan_host->chg_clk == host->mmc->ios.clock)
> + return 0;
> +
> + arasan_host->chg_clk = host->mmc->ios.clock;
> + if (host->mmc->ios.clock == 200000000)
> + clk = 0x0;
> + else if (host->mmc->ios.clock == 100000000)
> + clk = 0x2;
> + else if (host->mmc->ios.clock == 50000000)
> + clk = 0x1;
> + else
> + clk = 0x0;
> +
> + if (host->mmc_host_ops.hs400_enhanced_strobe) {
> + arasan_phy_set(host, ENHSTRB_MODE, 1, 0x0, 0x0,
> + DLLTRM_ICP, clk);
> + } else {
> + switch (host->mmc->ios.timing) {
> + case MMC_TIMING_LEGACY:
> + arasan_phy_set(host, LEGACY_MODE, 0x0, 0x0, 0x0,
> + 0x0, 0x0);
> + break;
> + case MMC_TIMING_MMC_HS:
> + case MMC_TIMING_SD_HS:
> + arasan_phy_set(host, HS_MODE, 0x3, 0x0, 0x2,
> + DLLTRM_ICP, clk);
> + break;
> + case MMC_TIMING_MMC_HS200:
> + case MMC_TIMING_UHS_SDR104:
> + arasan_phy_set(host, HS200_MODE, 0x2,
> + host->mmc->ios.drv_type, 0x0,
> + DLLTRM_ICP, clk);
> + break;
> + case MMC_TIMING_MMC_DDR52:
> + case MMC_TIMING_UHS_DDR50:
> + arasan_phy_set(host, DDR50_MODE, 0x1, 0x0,
> + 0x0, DLLTRM_ICP, clk);
> + break;
> + case MMC_TIMING_MMC_HS400:
> + arasan_phy_set(host, HS400_MODE, 0x1,
> + host->mmc->ios.drv_type, 0xa,
> + DLLTRM_ICP, clk);
> + break;
> + default:
> + break;
> + }
> + }
> + return 0;
> +}
> +
> +static int arasan_pci_probe_slot(struct sdhci_pci_slot *slot)
> +{
> + int err;
> +
> + slot->host->mmc->caps |= MMC_CAP_NONREMOVABLE | MMC_CAP_8_BIT_DATA;
> + err = arasan_phy_init(slot->host);
> + if (err)
> + return -ENODEV;
> + return 0;
> +}
> +
> +static void arasan_sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
> +{
> + sdhci_set_clock(host, clock);
> +
> + /* Change phy settings for the new clock */
> + arasan_select_phy_clock(host);
> +}
> +
> +static const struct sdhci_ops arasan_sdhci_pci_ops = {
> + .set_clock = arasan_sdhci_set_clock,
> + .enable_dma = sdhci_pci_enable_dma,
> + .set_bus_width = sdhci_set_bus_width,
> + .reset = sdhci_reset,
> + .set_uhs_signaling = sdhci_set_uhs_signaling,
> +};
> +
> +const struct sdhci_pci_fixes sdhci_arasan = {
> + .probe_slot = arasan_pci_probe_slot,
> + .ops = &arasan_sdhci_pci_ops,
> + .priv_size = sizeof(struct arasan_host),
> +};
> +
> diff --git a/drivers/mmc/host/sdhci-pci-core.c b/drivers/mmc/host/sdhci-pci-core.c
> index 3e4f04f..fc2859f 100644
> --- a/drivers/mmc/host/sdhci-pci-core.c
> +++ b/drivers/mmc/host/sdhci-pci-core.c
> @@ -33,7 +33,6 @@
> #include "sdhci.h"
> #include "sdhci-pci.h"
>
> -static int sdhci_pci_enable_dma(struct sdhci_host *host);
> static void sdhci_pci_hw_reset(struct sdhci_host *host);
>
> #ifdef CONFIG_PM_SLEEP
> @@ -1306,6 +1305,7 @@ static const struct pci_device_id pci_ids[] = {
> SDHCI_PCI_DEVICE(O2, SDS1, o2),
> SDHCI_PCI_DEVICE(O2, SEABIRD0, o2),
> SDHCI_PCI_DEVICE(O2, SEABIRD1, o2),
> + SDHCI_PCI_DEVICE(ARASAN, PHY_EMMC, arasan),
> SDHCI_PCI_DEVICE_CLASS(AMD, SYSTEM_SDHCI, PCI_CLASS_MASK, amd),
> /* Generic SD host controller */
> {PCI_DEVICE_CLASS(SYSTEM_SDHCI, PCI_CLASS_MASK)},
> @@ -1320,7 +1320,7 @@ MODULE_DEVICE_TABLE(pci, pci_ids);
> * *
> \*****************************************************************************/
>
> -static int sdhci_pci_enable_dma(struct sdhci_host *host)
> +int sdhci_pci_enable_dma(struct sdhci_host *host)
> {
> struct sdhci_pci_slot *slot;
> struct pci_dev *pdev;
> diff --git a/drivers/mmc/host/sdhci-pci.h b/drivers/mmc/host/sdhci-pci.h
> index 063506c..657d8c6 100644
> --- a/drivers/mmc/host/sdhci-pci.h
> +++ b/drivers/mmc/host/sdhci-pci.h
> @@ -54,6 +54,9 @@
>
> #define PCI_SUBDEVICE_ID_NI_7884 0x7884
>
> +#define PCI_VENDOR_ID_ARASAN 0x16e6
> +#define PCI_DEVICE_ID_ARASAN_PHY_EMMC 0x0670
The defines still don't line up. They should line up in the source code
when the tab width is set to 8.
> +
> /*
> * PCI device class and mask
> */
> @@ -169,11 +172,13 @@ static inline void *sdhci_pci_priv(struct sdhci_pci_slot *slot)
> #ifdef CONFIG_PM_SLEEP
> int sdhci_pci_resume_host(struct sdhci_pci_chip *chip);
> #endif
> -
> +int sdhci_pci_enable_dma(struct sdhci_host *host);
> int sdhci_pci_o2_probe_slot(struct sdhci_pci_slot *slot);
> int sdhci_pci_o2_probe(struct sdhci_pci_chip *chip);
> #ifdef CONFIG_PM_SLEEP
> int sdhci_pci_o2_resume(struct sdhci_pci_chip *chip);
> #endif
>
> +extern const struct sdhci_pci_fixes sdhci_arasan;
> +
> #endif /* __SDHCI_PCI_H */
>
^ permalink raw reply
* [PATCH v6 4/4] mmc: sdio support external scatter gather list
From: Xinming Hu @ 2017-12-19 12:06 UTC (permalink / raw)
To: Linux MMC
Cc: Ulf Hansson, Kalle Valo, Arend van Spriel, Franky Lin,
Hante Meuleman, Chi-Hsien Lin, Wright Feng, Zhiyuan Yang,
Tim Song, Cathy Luo, James Cao, Ganapathi Bhat, Xu Wang, Bob Tan,
Xinming Hu, Amitkumar Karwar
In-Reply-To: <1513685211-640-1-git-send-email-huxm@marvell.com>
Currently sdio device drivers need to prepare a big continuous
physical memory for large data transfer. mmc stack will construct
scatter/gather buffer list to make use of ADMA2.
According to the sdio host controller specification version 4.00
chapter 1.13.2, sdio device drivers have the ability to construct
scatter/gather DMA buffer list. In this way, they do not need to allocate
big memory.
This patch refactors current sdio command 53 wrapper mmc_io_rw_extended,
so that it can accept external scatter/gather buffer list from its caller,
such as the sdio device driver. This patch is very useful on some embedded
systems where large memory allocation fails sometimes. It gives 10 to 15%
better throughput performance compared to a case in which small single
data transfers are done.
Signed-off-by: Xinming Hu <huxm@marvell.com>
Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
---
v4: same as v3/v2/v1
v5: rebased on latest codebase
v6: same as v5
---
drivers/mmc/core/sdio_io.c | 22 ++++++++++++++++--
drivers/mmc/core/sdio_ops.c | 53 +++++++++++++++++++++++++++++++++++++++----
drivers/mmc/core/sdio_ops.h | 3 ++-
include/linux/mmc/sdio_func.h | 6 +++++
4 files changed, 76 insertions(+), 8 deletions(-)
diff --git a/drivers/mmc/core/sdio_io.c b/drivers/mmc/core/sdio_io.c
index 4806521..6046511 100644
--- a/drivers/mmc/core/sdio_io.c
+++ b/drivers/mmc/core/sdio_io.c
@@ -327,7 +327,7 @@ static int sdio_io_rw_ext_helper(struct sdio_func *func, int write,
size = blocks * func->cur_blksize;
ret = mmc_io_rw_extended(func->card, write,
- func->num, addr, incr_addr, buf,
+ func->num, addr, incr_addr, false, buf,
blocks, func->cur_blksize);
if (ret)
return ret;
@@ -345,7 +345,7 @@ static int sdio_io_rw_ext_helper(struct sdio_func *func, int write,
/* Indicate byte mode by setting "blocks" = 0 */
ret = mmc_io_rw_extended(func->card, write, func->num, addr,
- incr_addr, buf, 0, size);
+ incr_addr, false, buf, 0, size);
if (ret)
return ret;
@@ -513,6 +513,24 @@ int sdio_writesb(struct sdio_func *func, unsigned int addr, void *src,
}
EXPORT_SYMBOL_GPL(sdio_writesb);
+int sdio_readsb_sg_enh(struct sdio_func *func, unsigned int addr,
+ struct sg_table *dst)
+{
+ return mmc_io_rw_extended(func->card, 0, func->num,
+ addr, 0, true,
+ dst, 0, func->cur_blksize);
+}
+EXPORT_SYMBOL_GPL(sdio_readsb_sg_enh);
+
+int sdio_writesb_sg_enh(struct sdio_func *func, unsigned int addr,
+ struct sg_table *src)
+{
+ return mmc_io_rw_extended(func->card, 1, func->num,
+ addr, 0, true,
+ src, 0, func->cur_blksize);
+}
+EXPORT_SYMBOL_GPL(sdio_writesb_sg_enh);
+
/**
* sdio_readw - read a 16 bit integer from a SDIO function
* @func: SDIO function to access
diff --git a/drivers/mmc/core/sdio_ops.c b/drivers/mmc/core/sdio_ops.c
index abaaba3..2310a2a 100644
--- a/drivers/mmc/core/sdio_ops.c
+++ b/drivers/mmc/core/sdio_ops.c
@@ -115,16 +115,39 @@ int mmc_io_rw_direct(struct mmc_card *card, int write, unsigned fn,
return mmc_io_rw_direct_host(card->host, write, fn, addr, in, out);
}
+/**
+ * mmc_io_rw_extended - wrapper for sdio command 53 read/write operation
+ * @card: destination device for read/write
+ * @write: zero indcate read, non-zero indicate write.
+ * @fn: SDIO function to access
+ * @addr: address of (single byte) FIFO
+ * @incr_addr: set to 1 indicate read or write multiple bytes
+ of data to/from an IO register address that
+ increment by 1 after each operation.
+ * @sg_enhance: if set true, the caller of this function will
+ prepare scatter gather dma buffer list.
+ * @buf: buffer that contains the data to write. if sg_enhance
+ is set, it point to SG dma buffer list.
+ * @blocks: number of blocks of data transfer.
+ if set zero, indicate byte mode
+ if set non-zero, indicate block mode
+ if sg_enhance is set, this parameter will not be used.
+ * @blksz: block size for block mode data transfer.
+ *
+ */
int mmc_io_rw_extended(struct mmc_card *card, int write, unsigned fn,
- unsigned addr, int incr_addr, u8 *buf, unsigned blocks, unsigned blksz)
+ unsigned int addr, int incr_addr, bool sg_enhance,
+ void *buf, unsigned int blocks, unsigned int blksz)
{
struct mmc_request mrq = {};
struct mmc_command cmd = {};
struct mmc_data data = {};
struct scatterlist sg, *sg_ptr;
struct sg_table sgtable;
+ struct sg_table *sgtable_external;
unsigned int nents, left_size, i;
unsigned int seg_size = card->host->max_seg_size;
+ unsigned int sg_blocks = 0;
WARN_ON(blksz == 0);
@@ -140,16 +163,35 @@ int mmc_io_rw_extended(struct mmc_card *card, int write, unsigned fn,
cmd.arg |= fn << 28;
cmd.arg |= incr_addr ? 0x04000000 : 0x00000000;
cmd.arg |= addr << 9;
+ cmd.flags = MMC_RSP_SPI_R5 | MMC_RSP_R5 | MMC_CMD_ADTC;
+
+ data.flags = write ? MMC_DATA_WRITE : MMC_DATA_READ;
+ data.blksz = blksz;
+
+ if (sg_enhance) {
+ sgtable_external = buf;
+
+ for_each_sg(sgtable_external->sgl, sg_ptr,
+ sgtable_external->nents, i) {
+ if (sg_ptr->length > card->host->max_seg_size)
+ return -EINVAL;
+ sg_blocks += DIV_ROUND_UP(sg_ptr->length, blksz);
+ }
+
+ cmd.arg |= 0x08000000 | sg_blocks;
+ data.blocks = sg_blocks;
+ data.sg = sgtable_external->sgl;
+ data.sg_len = sgtable_external->nents;
+ goto mmc_data_req;
+ }
+
if (blocks == 0)
cmd.arg |= (blksz == 512) ? 0 : blksz; /* byte mode */
else
cmd.arg |= 0x08000000 | blocks; /* block mode */
- cmd.flags = MMC_RSP_SPI_R5 | MMC_RSP_R5 | MMC_CMD_ADTC;
- data.blksz = blksz;
/* Code in host drivers/fwk assumes that "blocks" always is >=1 */
data.blocks = blocks ? blocks : 1;
- data.flags = write ? MMC_DATA_WRITE : MMC_DATA_READ;
left_size = data.blksz * data.blocks;
nents = DIV_ROUND_UP(left_size, seg_size);
@@ -172,11 +214,12 @@ int mmc_io_rw_extended(struct mmc_card *card, int write, unsigned fn,
sg_init_one(&sg, buf, left_size);
}
+mmc_data_req:
mmc_set_data_timeout(&data, card);
mmc_wait_for_req(card->host, &mrq);
- if (nents > 1)
+ if (!sg_enhance && nents > 1)
sg_free_table(&sgtable);
if (cmd.error)
diff --git a/drivers/mmc/core/sdio_ops.h b/drivers/mmc/core/sdio_ops.h
index 96945ca..2d75100 100644
--- a/drivers/mmc/core/sdio_ops.h
+++ b/drivers/mmc/core/sdio_ops.h
@@ -23,7 +23,8 @@
int mmc_io_rw_direct(struct mmc_card *card, int write, unsigned fn,
unsigned addr, u8 in, u8* out);
int mmc_io_rw_extended(struct mmc_card *card, int write, unsigned fn,
- unsigned addr, int incr_addr, u8 *buf, unsigned blocks, unsigned blksz);
+ unsigned int addr, int incr_addr, bool sg_enhance,
+ void *buf, unsigned int blocks, unsigned int blksz);
int sdio_reset(struct mmc_host *host);
unsigned int mmc_align_data_size(struct mmc_card *card, unsigned int sz);
void sdio_irq_work(struct work_struct *work);
diff --git a/include/linux/mmc/sdio_func.h b/include/linux/mmc/sdio_func.h
index 72d4de4..d36ba47 100644
--- a/include/linux/mmc/sdio_func.h
+++ b/include/linux/mmc/sdio_func.h
@@ -14,6 +14,7 @@
#include <linux/device.h>
#include <linux/mod_devicetable.h>
+#include <linux/scatterlist.h>
#include <linux/mmc/pm.h>
@@ -136,6 +137,11 @@ extern int sdio_memcpy_fromio(struct sdio_func *func, void *dst,
extern int sdio_readsb(struct sdio_func *func, void *dst,
unsigned int addr, int count);
+int sdio_readsb_sg_enh(struct sdio_func *func, unsigned int addr,
+ struct sg_table *dst);
+int sdio_writesb_sg_enh(struct sdio_func *func, unsigned int addr,
+ struct sg_table *src);
+
extern void sdio_writeb(struct sdio_func *func, u8 b,
unsigned int addr, int *err_ret);
extern void sdio_writew(struct sdio_func *func, u16 b,
--
1.9.1
^ permalink raw reply related
* [PATCH v6 3/4] brcmfmac: change to use mmc api for accessing host supported maximum segment count and size
From: Xinming Hu @ 2017-12-19 12:06 UTC (permalink / raw)
To: Linux MMC
Cc: Ulf Hansson, Kalle Valo, Arend van Spriel, Franky Lin,
Hante Meuleman, Chi-Hsien Lin, Wright Feng, Zhiyuan Yang,
Tim Song, Cathy Luo, James Cao, Ganapathi Bhat, Xu Wang, Bob Tan,
Xinming Hu
In-Reply-To: <1513685211-640-1-git-send-email-huxm@marvell.com>
Using mmc standard api to get the host sg capability.
Signed-off-by: Xinming Hu <huxm@marvell.com>
---
v6: separate driver patch from patch 1/4.
---
drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
index cd58732..d0e08d0 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
@@ -886,19 +886,21 @@ void brcmf_sdiod_sgtable_alloc(struct brcmf_sdio_dev *sdiodev)
{
struct sdio_func *func;
struct mmc_host *host;
+ unsigned short max_segs = 0;
uint max_blocks;
uint nents;
int err;
func = sdiodev->func[2];
host = func->card->host;
- sdiodev->sg_support = host->max_segs > 1;
+
+ max_segs = sdio_get_host_max_seg_count(func);
+ sdiodev->sg_support = max_segs > 1;
max_blocks = min_t(uint, host->max_blk_count, 511u);
sdiodev->max_request_size = min_t(uint, host->max_req_size,
max_blocks * func->cur_blksize);
- sdiodev->max_segment_count = min_t(uint, host->max_segs,
- SG_MAX_SINGLE_ALLOC);
- sdiodev->max_segment_size = host->max_seg_size;
+ sdiodev->max_segment_count = min_t(uint, SG_MAX_SINGLE_ALLOC, max_segs);
+ sdiodev->max_segment_size = sdio_get_host_max_seg_size(func);
if (!sdiodev->sg_support)
return;
--
1.9.1
^ permalink raw reply related
* [PATCH v6 2/4] ath6kl: change to use mmc api for accessing host supported maximum segment count and size
From: Xinming Hu @ 2017-12-19 12:06 UTC (permalink / raw)
To: Linux MMC
Cc: Ulf Hansson, Kalle Valo, Arend van Spriel, Franky Lin,
Hante Meuleman, Chi-Hsien Lin, Wright Feng, Zhiyuan Yang,
Tim Song, Cathy Luo, James Cao, Ganapathi Bhat, Xu Wang, Bob Tan,
Xinming Hu
In-Reply-To: <1513685211-640-1-git-send-email-huxm@marvell.com>
Using mmc standard api to get the host sg capability.
Signed-off-by: Xinming Hu <huxm@marvell.com>
---
v6: separate driver patch from patch 1/4.
---
drivers/net/wireless/ath/ath6kl/sdio.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/net/wireless/ath/ath6kl/sdio.c b/drivers/net/wireless/ath/ath6kl/sdio.c
index 2195b1b..3217698 100644
--- a/drivers/net/wireless/ath/ath6kl/sdio.c
+++ b/drivers/net/wireless/ath/ath6kl/sdio.c
@@ -734,17 +734,18 @@ static int ath6kl_sdio_enable_scatter(struct ath6kl *ar)
struct htc_target *target = ar->htc_target;
int ret = 0;
bool virt_scat = false;
+ unsigned short max_segs = 0;
if (ar_sdio->scatter_enabled)
return 0;
ar_sdio->scatter_enabled = true;
+ max_segs = sdio_get_host_max_seg_count(ar_sdio->func);
/* check if host supports scatter and it meets our requirements */
- if (ar_sdio->func->card->host->max_segs < MAX_SCATTER_ENTRIES_PER_REQ) {
+ if (max_segs < MAX_SCATTER_ENTRIES_PER_REQ) {
ath6kl_err("host only supports scatter of :%d entries, need: %d\n",
- ar_sdio->func->card->host->max_segs,
- MAX_SCATTER_ENTRIES_PER_REQ);
+ max_segs, MAX_SCATTER_ENTRIES_PER_REQ);
virt_scat = true;
}
--
1.9.1
^ permalink raw reply related
* [PATCH v6 1/4] mmc: API for accessing host supported maximum segment count and size
From: Xinming Hu @ 2017-12-19 12:06 UTC (permalink / raw)
To: Linux MMC
Cc: Ulf Hansson, Kalle Valo, Arend van Spriel, Franky Lin,
Hante Meuleman, Chi-Hsien Lin, Wright Feng, Zhiyuan Yang,
Tim Song, Cathy Luo, James Cao, Ganapathi Bhat, Xu Wang, Bob Tan,
Xinming Hu, Amitkumar Karwar
sdio device drivers need be able to get the host supported max_segs
and max_seg_size, so that they know the buffer size to allocate while
utilizing the scatter/gather DMA buffer list.
This patch provides API for this purpose.
Signed-off-by: Xinming Hu <huxm@marvell.com>
Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
---
v2: v2 was submitted with minor improvement like replacing BUG_ON() with WARN_ON()
v3: Addressed below review comments from Ulf Hansson
a) In v3, patch has been split into two separate patches.
b) Patch 1/2 introduces an API to fetch max_seg_size and max_segs
c) Replaced WARN_ON() with proper error code when sg_ptr->length is invalid
d) Instead of duplicating the code in mmc_io_rw_extended(), extra bool parameter
has been added to this function and used it in new APIs for SG.
v4: Removed WARN_ON() calls in newly added APIs. It's gets called in probe handler.
Caller already takes care of it(Shawn Lin).
v5: Rebased on latest code base.
v6: Split driver caller to separate patch.
---
drivers/mmc/core/sdio_io.c | 21 +++++++++++++++++++++
include/linux/mmc/sdio_func.h | 3 +++
2 files changed, 24 insertions(+)
diff --git a/drivers/mmc/core/sdio_io.c b/drivers/mmc/core/sdio_io.c
index d40744b..4806521 100644
--- a/drivers/mmc/core/sdio_io.c
+++ b/drivers/mmc/core/sdio_io.c
@@ -725,3 +725,24 @@ int sdio_set_host_pm_flags(struct sdio_func *func, mmc_pm_flag_t flags)
return 0;
}
EXPORT_SYMBOL_GPL(sdio_set_host_pm_flags);
+
+/**
+ * sdio_get_host_max_seg_size - get host maximum segment size
+ * @func: SDIO function attached to host
+ */
+unsigned int sdio_get_host_max_seg_size(struct sdio_func *func)
+{
+ return func->card->host->max_seg_size;
+}
+EXPORT_SYMBOL_GPL(sdio_get_host_max_seg_size);
+
+/**
+ * sdio_get_host_max_seg_count - get host maximum segment count
+ * @func: SDIO function attached to host
+ */
+unsigned short sdio_get_host_max_seg_count(struct sdio_func *func)
+{
+ return func->card->host->max_segs;
+}
+EXPORT_SYMBOL_GPL(sdio_get_host_max_seg_count);
+
diff --git a/include/linux/mmc/sdio_func.h b/include/linux/mmc/sdio_func.h
index 97ca105..72d4de4 100644
--- a/include/linux/mmc/sdio_func.h
+++ b/include/linux/mmc/sdio_func.h
@@ -159,4 +159,7 @@ extern void sdio_f0_writeb(struct sdio_func *func, unsigned char b,
extern mmc_pm_flag_t sdio_get_host_pm_caps(struct sdio_func *func);
extern int sdio_set_host_pm_flags(struct sdio_func *func, mmc_pm_flag_t flags);
+unsigned short sdio_get_host_max_seg_count(struct sdio_func *func);
+unsigned int sdio_get_host_max_seg_size(struct sdio_func *func);
+
#endif /* LINUX_MMC_SDIO_FUNC_H */
--
1.9.1
^ permalink raw reply related
* Re: Re: [PATCH v5 1/2] mmc: API for accessing host supported maximum segment count and size
From: Xinming Hu @ 2017-12-19 12:06 UTC (permalink / raw)
To: Ulf Hansson
Cc: Linux MMC, Zhiyuan Yang, Tim Song, Cathy Luo, James Cao,
Ganapathi Bhat, Amitkumar Karwar
Hi Ulf,
> -----Original Message-----
> From: Ulf Hansson [mailto:ulf.hansson@linaro.org]
> Sent: 2017年12月19日 16:36
> To: Xinming Hu <huxm@marvell.com>
> Cc: Linux MMC <linux-mmc@vger.kernel.org>; Zhiyuan Yang
> <yangzy@marvell.com>; Tim Song <songtao@marvell.com>; Cathy Luo
> <cluo@marvell.com>; James Cao <jcao@marvell.com>; Ganapathi Bhat
> <gbhat@marvell.com>; Amitkumar Karwar <akarwar@marvell.com>
> Subject: [EXT] Re: [PATCH v5 1/2] mmc: API for accessing host supported
> maximum segment count and size
>
> External Email
>
> ----------------------------------------------------------------------
> On 22 November 2017 at 08:49, Xinming Hu <huxm@marvell.com> wrote:
> > sdio device drivers need be able to get the host supported max_segs
> > and max_seg_size, so that they know the buffer size to allocate while
> > utilizing the scatter/gather DMA buffer list.
> >
> > This patch provides API for this purpose.
>
> Apologize for the delay.
>
> Overall, this makes perfect sense to me. However some minor comment, see
> below.
>
Ok, great news !
> >
> > Signed-off-by: Xinming Hu <huxm@marvell.com>
> > Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
> > ---
> > v2: v2 was submitted with minor improvement like replacing BUG_ON()
> > with WARN_ON()
> > v3: Addressed below review comments from Ulf Hansson
> > a) In v3, patch has been split into two separate patches.
> > b) Patch 1/2 introduces an API to fetch max_seg_size and max_segs
> > c) Replaced WARN_ON() with proper error code when sg_ptr->length is
> invalid
> > d) Instead of duplicating the code in mmc_io_rw_extended(), extra bool
> parameter
> > has been added to this function and used it in new APIs for SG.
> > v4: Removed WARN_ON() calls in newly added APIs. It's gets called in probe
> handler.
> > Caller already takes care of it(Shawn Lin).
> > v5: Rebased on latest code base.
> > ---
> > drivers/mmc/core/sdio_io.c | 21
> +++++++++++++++++++++
> > .../wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c | 6 +++---
>
> The changes to the wifi driver, could please make that a separate patch on top
> of this one?
>
> There are actually a couple of more wireless drivers that should convert to use
> these new API, I would appreciate if you could fold in patches in the series for
> each of them as well (don't forget to cc correct people for them), then I intend
> to apply all of them via my tree, if no objections.
Okay, I will include the changes in v6.
>
> Otherwise this change looks good to me!
>
Thanks for the review!
Regards,
Simon
> [...]
>
> Kind regards
> Uffe
^ permalink raw reply
* Re: [PATCH 1/3] mmc: sdhci-pci-o2micro: Add hardware tuning for eMMC
From: Adrian Hunter @ 2017-12-19 9:05 UTC (permalink / raw)
To: LinuxPatchCommit, ulf.hansson@linaro.org,
linux-mmc@vger.kernel.org, linux-kernel@vger.kernel.org
Cc: Shirley Her (SC)
In-Reply-To: <CY4PR04MB047375B124358A33FB077C6EED3C0@CY4PR04MB0473.namprd04.prod.outlook.com>
On 04/12/17 11:39, LinuxPatchCommit wrote:
> Dear All,
>
> For O2micro/Bayhubtech SD Host DeviceID 8620, eMMC HS200 mode is working at 1.8v and it uses hardware tuning. The hardware tuning only needs to send one tuning command instead of multiple tuning commands with software tuning.
>
> Signed-off-by: ernest.zhang <ernest.zhang@bayhubtech.com>
> ---
> drivers/mmc/host/sdhci-pci-o2micro.c | 276 ++++++++++++++++++++++++++++++-----
> 1 file changed, 239 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-pci-o2micro.c b/drivers/mmc/host/sdhci-pci-o2micro.c
> index 14273ca00641..2a7ffd240497 100644
> --- a/drivers/mmc/host/sdhci-pci-o2micro.c
> +++ b/drivers/mmc/host/sdhci-pci-o2micro.c
> @@ -16,22 +16,223 @@
> */
>
> #include <linux/pci.h>
> -
> +#include <linux/platform_device.h>
> +#include <linux/regulator/fixed.h>
> +#include <linux/regulator/machine.h>
> +#include <linux/mmc/host.h>
> +#include <linux/mmc/mmc.h>
> +#include <linux/delay.h>
> #include "sdhci.h"
> #include "sdhci-pci.h"
> #include "sdhci-pci-o2micro.h"
>
> +static void sdhci_o2_start_tuning(struct sdhci_host *host) {
> + u16 ctrl;
> +
> + ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);
> + ctrl |= SDHCI_CTRL_EXEC_TUNING;
> + if (host->quirks2 & SDHCI_QUIRK2_TUNING_WORK_AROUND)
> + ctrl |= SDHCI_CTRL_TUNED_CLK;
> + sdhci_writew(host, ctrl, SDHCI_HOST_CONTROL2);
> +
> + /*
> + * As per the Host Controller spec v3.00, tuning command
> + * generates Buffer Read Ready interrupt, so enable that.
> + *
> + * Note: The spec clearly says that when tuning sequence
> + * is being performed, the controller does not generate
> + * interrupts other than Buffer Read Ready interrupt. But
> + * to make sure we don't hit a controller bug, we _only_
> + * enable Buffer Read Ready interrupt here.
> + */
> + sdhci_writel(host, SDHCI_INT_DATA_AVAIL, SDHCI_INT_ENABLE);
> + sdhci_writel(host, SDHCI_INT_DATA_AVAIL, SDHCI_SIGNAL_ENABLE); }
> +
> +static void sdhci_o2_end_tuning(struct sdhci_host *host) {
> + sdhci_writel(host, host->ier, SDHCI_INT_ENABLE);
> + sdhci_writel(host, host->ier, SDHCI_SIGNAL_ENABLE); }
> +
> +static inline bool sdhci_data_line_cmd(struct mmc_command *cmd) {
> + return cmd->data || cmd->flags & MMC_RSP_BUSY; }
> +
> +static void sdhci_del_timer(struct sdhci_host *host, struct mmc_request
> +*mrq) {
> + if (sdhci_data_line_cmd(mrq->cmd))
> + del_timer(&host->data_timer);
> + else
> + del_timer(&host->timer);
> +}
> +
> +static void sdhci_o2_set_tuning_mode(struct sdhci_host *host, bool hw)
> +{
> + u16 reg;
> +
> + if (hw) {
> + // enable hardware tuning
> + reg = sdhci_readw(host, O2_SD_VENDOR_SETTING);
> + reg &= (~O2_SD_HW_TUNING_ENABLE);
> + sdhci_writew(host, reg, O2_SD_VENDOR_SETTING);
> + } else {
> + reg = sdhci_readw(host, O2_SD_VENDOR_SETTING);
> + reg |= O2_SD_HW_TUNING_ENABLE;
> + sdhci_writew(host, reg, O2_SD_VENDOR_SETTING);
> + }
> +}
> +
> +static u8 data_buf[64];
> +
> +static void sdhci_o2_send_tuning(struct sdhci_host *host, u32 opcode) {
> + struct mmc_command cmd = { };
> + struct mmc_data data = { };
> + struct scatterlist sg;
> + struct mmc_request mrq = { };
> + unsigned long flags;
> + u32 b = host->sdma_boundary;
> + int size = sizeof(data_buf);
> +
> + cmd.opcode = opcode;
> + cmd.flags = MMC_RSP_PRESENT | MMC_RSP_OPCODE | MMC_RSP_CRC;
> + cmd.mrq = &mrq;
> + mrq.cmd = &cmd;
> + mrq.data = &data;
> + data.blksz = size;
> + data.blocks = 1;
> + data.flags = MMC_DATA_READ;
> +
> + data.timeout_ns = 150 * NSEC_PER_MSEC;
> +
> + data.sg = &sg;
> + data.sg_len = 1;
> + sg_init_one(&sg, data_buf, size);
> +
> + spin_lock_irqsave(&host->lock, flags);
> +
> + sdhci_writew(host, SDHCI_MAKE_BLKSZ(b, 64), SDHCI_BLOCK_SIZE);
> +
> + /*
> + * The tuning block is sent by the card to the host controller.
> + * So we set the TRNS_READ bit in the Transfer Mode register.
> + * This also takes care of setting DMA Enable and Multi Block
> + * Select in the same register to 0.
> + */
> + sdhci_writew(host, SDHCI_TRNS_READ, SDHCI_TRANSFER_MODE);
> +
> + sdhci_send_command(host, &cmd);
> +
> + host->cmd = NULL;
> +
> + sdhci_del_timer(host, &mrq);
> +
> + host->tuning_done = 0;
> +
> + mmiowb();
> + spin_unlock_irqrestore(&host->lock, flags);
> +
> + /* Wait for Buffer Read Ready interrupt */
> + wait_event_timeout(host->buf_ready_int, (host->tuning_done == 1),
> + msecs_to_jiffies(50));
> +
> +}
> +
> +static void sdhci_o2_reset_tuning(struct sdhci_host *host) {
> + u16 ctrl;
> +
> + ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);
> + ctrl &= ~SDHCI_CTRL_TUNED_CLK;
> + ctrl &= ~SDHCI_CTRL_EXEC_TUNING;
> + sdhci_writew(host, ctrl, SDHCI_HOST_CONTROL2); }
> +
> +static void __sdhci_o2_execute_tuning(struct sdhci_host *host, u32
> +opcode) {
> +
> + int i;
> +
> + sdhci_o2_send_tuning(host, MMC_SEND_TUNING_BLOCK_HS200);
So it looks like you send the tuning command only once. Is that right?
Maybe you could briefly outline what the tuning procedure is, and how it
differs from sdhci_execute_tuning().
> +
> + for (i = 0; i < 150; i++) {
> + u16 ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);
> +
> + if (!(ctrl & SDHCI_CTRL_EXEC_TUNING)) {
> + if (ctrl & SDHCI_CTRL_TUNED_CLK) {
> + pr_info("%s: HW tuning ok !\n",
> + mmc_hostname(host->mmc));
> + host->tuning_done = true;
> + return;
> + }
> + pr_warn("%s: HW tuning failed !\n",
> + mmc_hostname(host->mmc));
> +
> + break;
> + }
> +
> + mdelay(1);
> + }
> +
> + pr_info("%s: Tuning failed, falling back to fixed sampling clock\n",
> + mmc_hostname(host->mmc));
> + sdhci_o2_reset_tuning(host);
> +
> +}
> +
> +static int sdhci_o2_execute_tuning(struct mmc_host *mmc, u32 opcode) {
> + struct sdhci_host *host = mmc_priv(mmc);
> + int current_bus_width = 0;
> +
> + /*
> + * This handler only implements the eMMC tuning that is specific to
> + * this controller. Fall back to the standard method for other TINMING.
> + */
> + if (host->timing != MMC_TIMING_MMC_HS200)
> + return sdhci_execute_tuning(mmc, opcode);
> +
> + if (WARN_ON(opcode != MMC_SEND_TUNING_BLOCK_HS200))
> + return -EINVAL;
> +
> + /*
> + * o2 sdhci host didn't support 8bit emmc tuning
> + */
> + if (mmc->ios.bus_width == MMC_BUS_WIDTH_8) {
> + current_bus_width = mmc->ios.bus_width;
> + mmc->ios.bus_width = MMC_BUS_WIDTH_4;
> + mmc->ops->set_ios(mmc, &mmc->ios);
> + }
> +
> + sdhci_o2_set_tuning_mode(host, true);
> +
> + sdhci_o2_start_tuning(host);
> +
> + __sdhci_o2_execute_tuning(host, opcode);
> +
> + sdhci_o2_end_tuning(host);
> +
> + if (current_bus_width == MMC_BUS_WIDTH_8) {
> + mmc->ios.bus_width = current_bus_width;
> + mmc->ops->set_ios(mmc, &mmc->ios);
> + }
> +
> + host->flags &= ~SDHCI_HS400_TUNING;
> + return 0;
> +}
> +
> static void o2_pci_set_baseclk(struct sdhci_pci_chip *chip, u32 value) {
> u32 scratch_32;
> - pci_read_config_dword(chip->pdev,
> - O2_SD_PLL_SETTING, &scratch_32);
> + pci_read_config_dword(chip->pdev, O2_SD_PLL_SETTING, &scratch_32);
>
> scratch_32 &= 0x0000FFFF;
> scratch_32 |= value;
>
> - pci_write_config_dword(chip->pdev,
> - O2_SD_PLL_SETTING, scratch_32);
> + pci_write_config_dword(chip->pdev, O2_SD_PLL_SETTING, scratch_32);
> }
>
> static void o2_pci_led_enable(struct sdhci_pci_chip *chip) @@ -40,23 +241,19 @@ static void o2_pci_led_enable(struct sdhci_pci_chip *chip)
> u32 scratch_32;
>
> /* Set led of SD host function enable */
> - ret = pci_read_config_dword(chip->pdev,
> - O2_SD_FUNC_REG0, &scratch_32);
> + ret = pci_read_config_dword(chip->pdev, O2_SD_FUNC_REG0, &scratch_32);
> if (ret)
> return;
>
> scratch_32 &= ~O2_SD_FREG0_LEDOFF;
> - pci_write_config_dword(chip->pdev,
> - O2_SD_FUNC_REG0, scratch_32);
> + pci_write_config_dword(chip->pdev, O2_SD_FUNC_REG0, scratch_32);
>
> - ret = pci_read_config_dword(chip->pdev,
> - O2_SD_TEST_REG, &scratch_32);
> + ret = pci_read_config_dword(chip->pdev, O2_SD_TEST_REG, &scratch_32);
> if (ret)
> return;
>
> scratch_32 |= O2_SD_LED_ENABLE;
> - pci_write_config_dword(chip->pdev,
> - O2_SD_TEST_REG, scratch_32);
> + pci_write_config_dword(chip->pdev, O2_SD_TEST_REG, scratch_32);
>
> }
>
> @@ -104,8 +301,7 @@ static void sdhci_pci_o2_fujin2_pci_init(struct sdhci_pci_chip *chip)
> scratch_32 |= 0x00CC;
> pci_write_config_dword(chip->pdev, O2_SD_CAP_REG0, scratch_32);
> /* Set DLL Tuning Window */
> - ret = pci_read_config_dword(chip->pdev,
> - O2_SD_TUNING_CTRL, &scratch_32);
> + ret = pci_read_config_dword(chip->pdev, O2_SD_TUNING_CTRL,
> +&scratch_32);
> if (ret)
> return;
> scratch_32 &= ~(0x000000FF);
> @@ -137,8 +333,7 @@ static void sdhci_pci_o2_fujin2_pci_init(struct sdhci_pci_chip *chip)
> scratch_32 |= 0x30000000;
> pci_write_config_dword(chip->pdev, O2_SD_CAPS, scratch_32);
>
> - ret = pci_read_config_dword(chip->pdev,
> - O2_SD_MISC_CTRL4, &scratch_32);
> + ret = pci_read_config_dword(chip->pdev, O2_SD_MISC_CTRL4,
> +&scratch_32);
> if (ret)
> return;
> scratch_32 &= ~(0x000f0000);
> @@ -151,6 +346,7 @@ int sdhci_pci_o2_probe_slot(struct sdhci_pci_slot *slot)
> struct sdhci_pci_chip *chip;
> struct sdhci_host *host;
> u32 reg;
> + int ret;
>
> chip = slot->chip;
> host = slot->host;
> @@ -164,6 +360,22 @@ int sdhci_pci_o2_probe_slot(struct sdhci_pci_slot *slot)
> if (reg & 0x1)
> host->quirks |= SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12;
>
> + if (chip->pdev->device == PCI_DEVICE_ID_O2_SEABIRD0) {
> + ret = pci_read_config_dword(chip->pdev,
> + O2_SD_MISC_SETTING, ®);
> + if (ret)
> + return -EIO;
> + if (reg & (1 << 4)) {
> + pr_info("%s: emmc 1.8v flag is set,
> + force 1.8v signaling voltage\n",
> + mmc_hostname(host->mmc));
> + host->flags &= ~(SDHCI_SIGNALING_330);
> + host->flags |= SDHCI_SIGNALING_180;
> + }
> + }
> +
> + host->mmc_host_ops.execute_tuning = sdhci_o2_execute_tuning;
> +
> if (chip->pdev->device != PCI_DEVICE_ID_O2_FUJIN2)
> break;
> /* set dll watch dog timer */
> @@ -191,8 +403,7 @@ int sdhci_pci_o2_probe(struct sdhci_pci_chip *chip)
> case PCI_DEVICE_ID_O2_8320:
> case PCI_DEVICE_ID_O2_8321:
> /* This extra setup is required due to broken ADMA. */
> - ret = pci_read_config_byte(chip->pdev,
> - O2_SD_LOCK_WP, &scratch);
> + ret = pci_read_config_byte(chip->pdev, O2_SD_LOCK_WP, &scratch);
> if (ret)
> return ret;
> scratch &= 0x7f;
> @@ -202,8 +413,7 @@ int sdhci_pci_o2_probe(struct sdhci_pci_chip *chip)
> pci_write_config_byte(chip->pdev, O2_SD_MULTI_VCC3V, 0x08);
>
> /* Disable CLK_REQ# support after media DET */
> - ret = pci_read_config_byte(chip->pdev,
> - O2_SD_CLKREQ, &scratch);
> + ret = pci_read_config_byte(chip->pdev, O2_SD_CLKREQ, &scratch);
> if (ret)
> return ret;
> scratch |= 0x20;
> @@ -224,16 +434,14 @@ int sdhci_pci_o2_probe(struct sdhci_pci_chip *chip)
> pci_write_config_byte(chip->pdev, O2_SD_ADMA2, 0x08);
>
> /* Disable the infinite transfer mode */
> - ret = pci_read_config_byte(chip->pdev,
> - O2_SD_INF_MOD, &scratch);
> + ret = pci_read_config_byte(chip->pdev, O2_SD_INF_MOD, &scratch);
> if (ret)
> return ret;
> scratch |= 0x08;
> pci_write_config_byte(chip->pdev, O2_SD_INF_MOD, scratch);
>
> /* Lock WP */
> - ret = pci_read_config_byte(chip->pdev,
> - O2_SD_LOCK_WP, &scratch);
> + ret = pci_read_config_byte(chip->pdev, O2_SD_LOCK_WP, &scratch);
> if (ret)
> return ret;
> scratch |= 0x80;
> @@ -243,8 +451,7 @@ int sdhci_pci_o2_probe(struct sdhci_pci_chip *chip)
> case PCI_DEVICE_ID_O2_SDS1:
> case PCI_DEVICE_ID_O2_FUJIN2:
> /* UnLock WP */
> - ret = pci_read_config_byte(chip->pdev,
> - O2_SD_LOCK_WP, &scratch);
> + ret = pci_read_config_byte(chip->pdev, O2_SD_LOCK_WP, &scratch);
> if (ret)
> return ret;
>
> @@ -319,15 +526,13 @@ int sdhci_pci_o2_probe(struct sdhci_pci_chip *chip)
> if (ret)
> return ret;
> scratch_32 &= ~(0xE0);
> - pci_write_config_dword(chip->pdev,
> - O2_SD_CAP_REG2, scratch_32);
> + pci_write_config_dword(chip->pdev, O2_SD_CAP_REG2, scratch_32);
>
> if (chip->pdev->device == PCI_DEVICE_ID_O2_FUJIN2)
> sdhci_pci_o2_fujin2_pci_init(chip);
>
> /* Lock WP */
> - ret = pci_read_config_byte(chip->pdev,
> - O2_SD_LOCK_WP, &scratch);
> + ret = pci_read_config_byte(chip->pdev, O2_SD_LOCK_WP, &scratch);
> if (ret)
> return ret;
> scratch |= 0x80;
> @@ -336,8 +541,7 @@ int sdhci_pci_o2_probe(struct sdhci_pci_chip *chip)
> case PCI_DEVICE_ID_O2_SEABIRD0:
> case PCI_DEVICE_ID_O2_SEABIRD1:
> /* UnLock WP */
> - ret = pci_read_config_byte(chip->pdev,
> - O2_SD_LOCK_WP, &scratch);
> + ret = pci_read_config_byte(chip->pdev, O2_SD_LOCK_WP, &scratch);
> if (ret)
> return ret;
>
> @@ -369,11 +573,9 @@ int sdhci_pci_o2_probe(struct sdhci_pci_chip *chip)
> }
>
> /* Set Tuning Windows to 5 */
> - pci_write_config_byte(chip->pdev,
> - O2_SD_TUNING_CTRL, 0x55);
> + pci_write_config_byte(chip->pdev, O2_SD_TUNING_CTRL, 0x55);
> /* Lock WP */
> - ret = pci_read_config_byte(chip->pdev,
> - O2_SD_LOCK_WP, &scratch);
> + ret = pci_read_config_byte(chip->pdev, O2_SD_LOCK_WP, &scratch);
> if (ret)
> return ret;
> scratch |= 0x80;
> --
> 2.14.1
>
^ permalink raw reply
* Re: [PATCH 3/3] mmc: sdhci: Modify sdhci o2 quirk for eMMC HS200 tuning case
From: Adrian Hunter @ 2017-12-19 8:37 UTC (permalink / raw)
To: LinuxPatchCommit, ulf.hansson@linaro.org,
linux-mmc@vger.kernel.org, linux-kernel@vger.kernel.org
Cc: Shirley Her (SC)
In-Reply-To: <CY4PR04MB047345A73648C7A212D505D7ED3C0@CY4PR04MB0473.namprd04.prod.outlook.com>
On 04/12/17 12:10, LinuxPatchCommit wrote:
> Dear all,
>
> Don't clear transfer mode register in sdhci_set_transfer_mode().
>
> In sdhci_set_transfer_mode(), clear transfer mode quirk2 will clear SD host transfer mode register for non-data commands. This quirk is used for the bug of O2micro/Bayhubtech devices. In eMMC HS200 tuning case, the tuning process uses hardware tuning and it needs to set transfer mode register for tuning command.
>
> Signed-off-by: ernest.zhang <ernest.zhang@bayhubtech.com>
> ---
> drivers/mmc/host/sdhci.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c index 0d5fcca18c9e..d5b19fc9ea56 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -918,7 +918,9 @@ static void sdhci_set_transfer_mode(struct sdhci_host *host,
> if (data == NULL) {
> if (host->quirks2 &
> SDHCI_QUIRK2_CLEAR_TRANSFERMODE_REG_BEFORE_CMD) {
> - sdhci_writew(host, 0x0, SDHCI_TRANSFER_MODE);
> + /* cannot clear transfer mode register when tuning */
> + if (cmd->opcode != MMC_SEND_TUNING_BLOCK_HS200)
> + sdhci_writew(host, 0x0, SDHCI_TRANSFER_MODE);
In the HS200 case you are sending the tuning command with data, so I do not
understand why you need this i.e. you will never get here
> } else {
> /* clear Auto CMD settings for no data CMDs */
> mode = sdhci_readw(host, SDHCI_TRANSFER_MODE);
> --
> 2.14.1
>
^ permalink raw reply
* Re: [PATCH v5 1/2] mmc: API for accessing host supported maximum segment count and size
From: Ulf Hansson @ 2017-12-19 8:36 UTC (permalink / raw)
To: Xinming Hu
Cc: Linux MMC, Zhiyuan Yang, Tim Song, Cathy Luo, James Cao,
Ganapathi Bhat, Amitkumar Karwar
In-Reply-To: <1511336987-21263-1-git-send-email-huxm@marvell.com>
On 22 November 2017 at 08:49, Xinming Hu <huxm@marvell.com> wrote:
> sdio device drivers need be able to get the host supported max_segs
> and max_seg_size, so that they know the buffer size to allocate while
> utilizing the scatter/gather DMA buffer list.
>
> This patch provides API for this purpose.
Apologize for the delay.
Overall, this makes perfect sense to me. However some minor comment, see below.
>
> Signed-off-by: Xinming Hu <huxm@marvell.com>
> Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
> ---
> v2: v2 was submitted with minor improvement like replacing BUG_ON() with WARN_ON()
> v3: Addressed below review comments from Ulf Hansson
> a) In v3, patch has been split into two separate patches.
> b) Patch 1/2 introduces an API to fetch max_seg_size and max_segs
> c) Replaced WARN_ON() with proper error code when sg_ptr->length is invalid
> d) Instead of duplicating the code in mmc_io_rw_extended(), extra bool parameter
> has been added to this function and used it in new APIs for SG.
> v4: Removed WARN_ON() calls in newly added APIs. It's gets called in probe handler.
> Caller already takes care of it(Shawn Lin).
> v5: Rebased on latest code base.
> ---
> drivers/mmc/core/sdio_io.c | 21 +++++++++++++++++++++
> .../wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c | 6 +++---
The changes to the wifi driver, could please make that a separate
patch on top of this one?
There are actually a couple of more wireless drivers that should
convert to use these new API, I would appreciate if you could fold in
patches in the series for each of them as well (don't forget to cc
correct people for them), then I intend to apply all of them via my
tree, if no objections.
Otherwise this change looks good to me!
[...]
Kind regards
Uffe
^ permalink raw reply
* Re: [PATCH 2/3] mmc: sdhci-pci-o2micro: Add eMMC HS200 support
From: Adrian Hunter @ 2017-12-19 8:02 UTC (permalink / raw)
To: LinuxPatchCommit, ulf.hansson@linaro.org,
linux-mmc@vger.kernel.org, linux-kernel@vger.kernel.org
Cc: Shirley Her (SC)
In-Reply-To: <CY4PR04MB047374AC40568EF03031F49CED3C0@CY4PR04MB0473.namprd04.prod.outlook.com>
On 04/12/17 12:10, LinuxPatchCommit wrote:
> Dear all,
A greeting is not needed in a commit message.
>
> Add register definition for eMMC HS200 mode.
> Add hardware tuning for eMMC HS200 mode.
>
> Signed-off-by: ernest.zhang <ernest.zhang@bayhubtech.com>
> ---
> drivers/mmc/host/sdhci-pci-o2micro.h | 3 +++
sdhci-pci-o2micro.h was removed by commit 361eeda0ca16 ("mmc: sdhci-pci:
Tidy o2micro definitions"). Please re-base.
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci-pci-o2micro.h b/drivers/mmc/host/sdhci-pci-o2micro.h
> index 770f53857211..3fb957729315 100644
> --- a/drivers/mmc/host/sdhci-pci-o2micro.h
> +++ b/drivers/mmc/host/sdhci-pci-o2micro.h
> @@ -49,6 +49,7 @@
> #define O2_SD_MISC_CTRL4 0xFC
> #define O2_SD_TUNING_CTRL 0x300
> #define O2_SD_PLL_SETTING 0x304
> +#define O2_SD_MISC_SETTING 0x308
> #define O2_SD_CLK_SETTING 0x328
> #define O2_SD_CAP_REG2 0x330
> #define O2_SD_CAP_REG0 0x334
> @@ -62,6 +63,8 @@
> #define O2_SD_FREG4_ENABLE_CLK_SET BIT(22)
>
> #define O2_SD_VENDOR_SETTING 0x110
> +#define O2_SD_HW_TUNING_ENABLE BIT(4)
Both these definitions are needed in patch 1. It would be better to add
them in that patch.
> +
> #define O2_SD_VENDOR_SETTING2 0x1C8
>
> extern int sdhci_pci_o2_probe_slot(struct sdhci_pci_slot *slot);
>
^ permalink raw reply
* Re: [PATCH v4] mmc: sdhci-xenon: wait 5ms after set 1.8V signal enable
From: Ulf Hansson @ 2017-12-19 7:54 UTC (permalink / raw)
To: Zhoujie Wu
Cc: Adrian Hunter, linux-mmc@vger.kernel.org, Nadav Haklai, Victor Gu,
xswang, Wilson Ding, Kostya Porotchkin, Hanna Hawa, hongd,
Doug Jones, Ryan Gao, Wei(SOCP) Liu, Gregory Clement,
Thomas Petazzoni
In-Reply-To: <1513636727-4503-1-git-send-email-zjwu@marvell.com>
On 18 December 2017 at 23:38, Zhoujie Wu <zjwu@marvell.com> wrote:
> According to SD spec 3.00 3.6.1 signal voltage switch
> procedure step 6~8,
> (6) Set 1.8V Signal Enable in the Host Control 2 register.
> (7) Wait 5ms. 1.8V voltage regulator shall be stable within this period.
> (8) If 1.8V Signal Enable is cleared by Host Controller, go to step (12).
> Host should wait 5ms after set 1.8V signal enable bit in
> Host Control 2 register and check if 1.8V is stable or not.
>
> But current code checks this bit right after set it.
> On some platforms with xenon controller found the bit is
> cleared right away and host reports "1.8V regulator output
> did not became stable" and 5ms delay can help.
>
> Implement voltage_switch callback for xenon controller to add 5ms
> delay to make sure the 1.8V signal enable bit is set by controller.
>
> Signed-off-by: Zhoujie Wu <zjwu@marvell.com>
Thanks, applied for next!
Kind regards
Uffe
> ---
> v4: move the 5ms delay from sdhci.c to xenon driver.
> v3: give more details in comments.
> v2: remove undeliverable cc list email.
>
> drivers/mmc/host/sdhci-xenon.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci-xenon.c b/drivers/mmc/host/sdhci-xenon.c
> index 0842bbc..4d0791f 100644
> --- a/drivers/mmc/host/sdhci-xenon.c
> +++ b/drivers/mmc/host/sdhci-xenon.c
> @@ -230,7 +230,14 @@ static void xenon_set_power(struct sdhci_host *host, unsigned char mode,
> mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd);
> }
>
> +static void xenon_voltage_switch(struct sdhci_host *host)
> +{
> + /* Wait for 5ms after set 1.8V signal enable bit */
> + usleep_range(5000, 5500);
> +}
> +
> static const struct sdhci_ops sdhci_xenon_ops = {
> + .voltage_switch = xenon_voltage_switch,
> .set_clock = sdhci_set_clock,
> .set_power = xenon_set_power,
> .set_bus_width = sdhci_set_bus_width,
> --
> 1.9.1
>
^ permalink raw reply
* Re: [PATCH v2 00/22] mmc: tmio: various fixes and cleanups
From: Ulf Hansson @ 2017-12-19 7:54 UTC (permalink / raw)
To: Masahiro Yamada
Cc: linux-mmc@vger.kernel.org, Wolfram Sang, Simon Horman,
Yoshihiro Shimoda, Linux-Renesas, linux-kernel@vger.kernel.org
In-Reply-To: <CAK7LNASdJz9LpcCGQJjHU_CkxtXKhTbnRYA2Caq8Ck9OPLHghg@mail.gmail.com>
On 19 December 2017 at 04:56, Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
> Hi Ulf,
>
>
> 2017-12-15 18:18 GMT+09:00 Ulf Hansson <ulf.hansson@linaro.org>:
>> On 24 November 2017 at 17:24, Masahiro Yamada
>> <yamada.masahiro@socionext.com> wrote:
>>>
>>> I am working on this IP for Socionext SoCs.
>>>
>>> I was hit by several issues, and noticed various
>>> clean-up candidates.
>>>
>>> - Fix and clean-up Kconfig
>>> - Fix various card detection problems
>>> - Move Renesas private data out of TMIO core
>>> - Allow to perform platform-specific settings before MMC host starts
>>> - Fix weird IRQ handling
>>>
>>> I am getting more and more patches for TMIO.
>>> I put all in a single series to clarify the patch order.
>>>
>>> 1, 2, 4, 5, 6, 7 were already acked or reviewed by Wolfram Sang.
>>>
>>>
>>> Masahiro Yamada (22):
>>> mmc: renesas_sdhi: consolidate DMAC CONFIG options
>>> mmc: renesas_sdhi: remove wrong depends on to enable compile test
>>> mmc: renesas_sdhi: remove eprobe jump label
>>> mmc: tmio: set tmio_mmc_host to driver data
>>> mmc: tmio: use devm_ioremap_resource() instead of devm_ioremap()
>>> mmc: tmio: move mmc_host_ops to struct tmio_mmc_host from static data
>>> mmc: tmio, renesas_sdhi: set mmc_host_ops hooks directly
>>> mmc: tmio: move mmc_gpio_request_cd() before mmc_add_host()
>>> mmc: tmio: use mmc_can_gpio_cd() instead of checking
>>> TMIO_MMC_USE_GPIO_CD
>>> mmc: tmio: support IP-builtin card detection logic
>>> mmc: renesas_sdhi: remove always false condition
>>> mmc: tmio,renesas_sdhi: move struct tmio_mmc_dma to renesas_sdhi.h
>>> mmc: tmio,renesas_sdhi: move Renesas-specific DMA data to
>>> renesas_sdhi.h
>>> mmc: tmio,renesas_sdhi: move ssc_tappos to renesas_sdhi.h
>>> mmc: tmio: change bus_shift to unsigned int
>>> mmc: tmio: fix never-detected card insertion bug
>>> mmc: tmio: move TMIO_MASK_{READOP,WRITEOP} handling to correct place
>>> mmc: tmio: remove useless TMIO_MASK_CMD handling in
>>> tmio_mmc_host_probe()
>>> mmc: tmio: ioremap memory resource in tmio_mmc_host_alloc()
>>> mmc: tmio: move clk_enable/disable out of tmio_mmc_host_probe()
>>> mmc: tmio: move {tmio_}mmc_of_parse() to tmio_mmc_host_alloc()
>>> mmc: tmio: remove dma_ops from tmio_mmc_host_probe() argument
>>>
>>> drivers/mmc/host/Kconfig | 5 +-
>>> drivers/mmc/host/Makefile | 8 +-
>>> drivers/mmc/host/renesas_sdhi.h | 22 ++++
>>> drivers/mmc/host/renesas_sdhi_core.c | 49 ++++-----
>>> drivers/mmc/host/renesas_sdhi_internal_dmac.c | 14 ++-
>>> drivers/mmc/host/renesas_sdhi_sys_dmac.c | 35 +++---
>>> drivers/mmc/host/tmio_mmc.c | 23 ++--
>>> drivers/mmc/host/tmio_mmc.h | 23 +---
>>> drivers/mmc/host/tmio_mmc_core.c | 149 +++++++++++++-------------
>>> 9 files changed, 170 insertions(+), 158 deletions(-)
>>>
>>> --
>>> 2.7.4
>>>
>>
>> To get this moving, I have applied patch 1->8 for next, thanks!
>
>
> Could you apply 11->15 as well?
> They were reviewed by Wolfram.
Applied for next!
>
> We can skip 9, 10.
Okay, thought there were a dependency.
Thanks and kind regards
Uffe
^ permalink raw reply
* Re: [PATCH v4] mmc: sdhci-xenon: wait 5ms after set 1.8V signal enable
From: Adrian Hunter @ 2017-12-19 7:24 UTC (permalink / raw)
To: Zhoujie Wu, ulf.hansson, linux-mmc
Cc: nadavh, xigu, xswang, dingwei, kostap, hannah, hongd, dougj, ygao,
liuw, gregory.clement, thomas.petazzoni
In-Reply-To: <1513636727-4503-1-git-send-email-zjwu@marvell.com>
On 19/12/17 00:38, Zhoujie Wu wrote:
> According to SD spec 3.00 3.6.1 signal voltage switch
> procedure step 6~8,
> (6) Set 1.8V Signal Enable in the Host Control 2 register.
> (7) Wait 5ms. 1.8V voltage regulator shall be stable within this period.
> (8) If 1.8V Signal Enable is cleared by Host Controller, go to step (12).
> Host should wait 5ms after set 1.8V signal enable bit in
> Host Control 2 register and check if 1.8V is stable or not.
>
> But current code checks this bit right after set it.
> On some platforms with xenon controller found the bit is
> cleared right away and host reports "1.8V regulator output
> did not became stable" and 5ms delay can help.
>
> Implement voltage_switch callback for xenon controller to add 5ms
> delay to make sure the 1.8V signal enable bit is set by controller.
>
> Signed-off-by: Zhoujie Wu <zjwu@marvell.com>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
> v4: move the 5ms delay from sdhci.c to xenon driver.
> v3: give more details in comments.
> v2: remove undeliverable cc list email.
>
> drivers/mmc/host/sdhci-xenon.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci-xenon.c b/drivers/mmc/host/sdhci-xenon.c
> index 0842bbc..4d0791f 100644
> --- a/drivers/mmc/host/sdhci-xenon.c
> +++ b/drivers/mmc/host/sdhci-xenon.c
> @@ -230,7 +230,14 @@ static void xenon_set_power(struct sdhci_host *host, unsigned char mode,
> mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd);
> }
>
> +static void xenon_voltage_switch(struct sdhci_host *host)
> +{
> + /* Wait for 5ms after set 1.8V signal enable bit */
> + usleep_range(5000, 5500);
> +}
> +
> static const struct sdhci_ops sdhci_xenon_ops = {
> + .voltage_switch = xenon_voltage_switch,
> .set_clock = sdhci_set_clock,
> .set_power = xenon_set_power,
> .set_bus_width = sdhci_set_bus_width,
>
^ permalink raw reply
* Re: [PATCH v2 00/22] mmc: tmio: various fixes and cleanups
From: Masahiro Yamada @ 2017-12-19 3:56 UTC (permalink / raw)
To: Ulf Hansson
Cc: linux-mmc@vger.kernel.org, Wolfram Sang, Simon Horman,
Yoshihiro Shimoda, Linux-Renesas, linux-kernel@vger.kernel.org
In-Reply-To: <CAPDyKFqE7Pg8fJFyOtihdRnma+dFQd3+YPWGn6pXbpzhpERK9Q@mail.gmail.com>
Hi Ulf,
2017-12-15 18:18 GMT+09:00 Ulf Hansson <ulf.hansson@linaro.org>:
> On 24 November 2017 at 17:24, Masahiro Yamada
> <yamada.masahiro@socionext.com> wrote:
>>
>> I am working on this IP for Socionext SoCs.
>>
>> I was hit by several issues, and noticed various
>> clean-up candidates.
>>
>> - Fix and clean-up Kconfig
>> - Fix various card detection problems
>> - Move Renesas private data out of TMIO core
>> - Allow to perform platform-specific settings before MMC host starts
>> - Fix weird IRQ handling
>>
>> I am getting more and more patches for TMIO.
>> I put all in a single series to clarify the patch order.
>>
>> 1, 2, 4, 5, 6, 7 were already acked or reviewed by Wolfram Sang.
>>
>>
>> Masahiro Yamada (22):
>> mmc: renesas_sdhi: consolidate DMAC CONFIG options
>> mmc: renesas_sdhi: remove wrong depends on to enable compile test
>> mmc: renesas_sdhi: remove eprobe jump label
>> mmc: tmio: set tmio_mmc_host to driver data
>> mmc: tmio: use devm_ioremap_resource() instead of devm_ioremap()
>> mmc: tmio: move mmc_host_ops to struct tmio_mmc_host from static data
>> mmc: tmio, renesas_sdhi: set mmc_host_ops hooks directly
>> mmc: tmio: move mmc_gpio_request_cd() before mmc_add_host()
>> mmc: tmio: use mmc_can_gpio_cd() instead of checking
>> TMIO_MMC_USE_GPIO_CD
>> mmc: tmio: support IP-builtin card detection logic
>> mmc: renesas_sdhi: remove always false condition
>> mmc: tmio,renesas_sdhi: move struct tmio_mmc_dma to renesas_sdhi.h
>> mmc: tmio,renesas_sdhi: move Renesas-specific DMA data to
>> renesas_sdhi.h
>> mmc: tmio,renesas_sdhi: move ssc_tappos to renesas_sdhi.h
>> mmc: tmio: change bus_shift to unsigned int
>> mmc: tmio: fix never-detected card insertion bug
>> mmc: tmio: move TMIO_MASK_{READOP,WRITEOP} handling to correct place
>> mmc: tmio: remove useless TMIO_MASK_CMD handling in
>> tmio_mmc_host_probe()
>> mmc: tmio: ioremap memory resource in tmio_mmc_host_alloc()
>> mmc: tmio: move clk_enable/disable out of tmio_mmc_host_probe()
>> mmc: tmio: move {tmio_}mmc_of_parse() to tmio_mmc_host_alloc()
>> mmc: tmio: remove dma_ops from tmio_mmc_host_probe() argument
>>
>> drivers/mmc/host/Kconfig | 5 +-
>> drivers/mmc/host/Makefile | 8 +-
>> drivers/mmc/host/renesas_sdhi.h | 22 ++++
>> drivers/mmc/host/renesas_sdhi_core.c | 49 ++++-----
>> drivers/mmc/host/renesas_sdhi_internal_dmac.c | 14 ++-
>> drivers/mmc/host/renesas_sdhi_sys_dmac.c | 35 +++---
>> drivers/mmc/host/tmio_mmc.c | 23 ++--
>> drivers/mmc/host/tmio_mmc.h | 23 +---
>> drivers/mmc/host/tmio_mmc_core.c | 149 +++++++++++++-------------
>> 9 files changed, 170 insertions(+), 158 deletions(-)
>>
>> --
>> 2.7.4
>>
>
> To get this moving, I have applied patch 1->8 for next, thanks!
Could you apply 11->15 as well?
They were reviewed by Wolfram.
We can skip 9, 10.
--
Best Regards
Masahiro Yamada
^ permalink raw reply
* [PATCH v4] mmc: sdhci-xenon: wait 5ms after set 1.8V signal enable
From: Zhoujie Wu @ 2017-12-18 22:38 UTC (permalink / raw)
To: ulf.hansson, adrian.hunter, linux-mmc
Cc: nadavh, xigu, xswang, dingwei, kostap, hannah, hongd, dougj, ygao,
liuw, gregory.clement, thomas.petazzoni, Zhoujie Wu
According to SD spec 3.00 3.6.1 signal voltage switch
procedure step 6~8,
(6) Set 1.8V Signal Enable in the Host Control 2 register.
(7) Wait 5ms. 1.8V voltage regulator shall be stable within this period.
(8) If 1.8V Signal Enable is cleared by Host Controller, go to step (12).
Host should wait 5ms after set 1.8V signal enable bit in
Host Control 2 register and check if 1.8V is stable or not.
But current code checks this bit right after set it.
On some platforms with xenon controller found the bit is
cleared right away and host reports "1.8V regulator output
did not became stable" and 5ms delay can help.
Implement voltage_switch callback for xenon controller to add 5ms
delay to make sure the 1.8V signal enable bit is set by controller.
Signed-off-by: Zhoujie Wu <zjwu@marvell.com>
---
v4: move the 5ms delay from sdhci.c to xenon driver.
v3: give more details in comments.
v2: remove undeliverable cc list email.
drivers/mmc/host/sdhci-xenon.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/mmc/host/sdhci-xenon.c b/drivers/mmc/host/sdhci-xenon.c
index 0842bbc..4d0791f 100644
--- a/drivers/mmc/host/sdhci-xenon.c
+++ b/drivers/mmc/host/sdhci-xenon.c
@@ -230,7 +230,14 @@ static void xenon_set_power(struct sdhci_host *host, unsigned char mode,
mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd);
}
+static void xenon_voltage_switch(struct sdhci_host *host)
+{
+ /* Wait for 5ms after set 1.8V signal enable bit */
+ usleep_range(5000, 5500);
+}
+
static const struct sdhci_ops sdhci_xenon_ops = {
+ .voltage_switch = xenon_voltage_switch,
.set_clock = sdhci_set_clock,
.set_power = xenon_set_power,
.set_bus_width = sdhci_set_bus_width,
--
1.9.1
^ permalink raw reply related
* Re: [EXT] Re: [PATCH v3] mmc: sdhci: wait 5ms after set 1.8V signal enable
From: Zhoujie Wu @ 2017-12-18 22:26 UTC (permalink / raw)
To: Adrian Hunter, ulf.hansson, linux-mmc
Cc: nadavh, xigu, xswang, dingwei, kostap, hannah, hongd, dougj, ygao,
liuw, gregory.clement, thomas.petazzoni
In-Reply-To: <472f0d18-e529-2477-4809-079a5970a362@intel.com>
On 12/18/2017 05:19 AM, Adrian Hunter wrote:
> External Email
>
> ----------------------------------------------------------------------
> On 15/12/17 21:28, Zhoujie Wu wrote:
>> According to SD spec 3.00 3.6.1 signal voltage switch
>> procedure step 6~8,
>> (6) Set 1.8V Signal Enable in the Host Control 2 register.
>> (7) Wait 5ms. 1.8V voltage regulator shall be stable within this period.
>> (8) If 1.8V Signal Enable is cleared by Host Controller, go to step (12).
>>
>> Host should wait 5ms after set 1.8V signal enable bit in
>> Host Control 2 register and check if 1.8V is stable or not.
>>
>> But current code checks if this bit is cleared by controller
>> right after set it. On some platforms found the bit is not
>> cleared right away and host reports "1.8V regulator output
>> did not became stable" and 5ms delay can help.
> The check is for the bit set not cleared.
I will correct the comments.
>> Follow the spec and add 5ms delay to make sure the 1.8V Signal Enable
>> bit is cleared.
>>
>> Signed-off-by: Zhoujie Wu <zjwu@marvell.com>
>> ---
>> drivers/mmc/host/sdhci.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>> index e9290a3..fe5f208 100644
>> --- a/drivers/mmc/host/sdhci.c
>> +++ b/drivers/mmc/host/sdhci.c
>> @@ -1933,6 +1933,9 @@ int sdhci_start_signal_voltage_switch(struct mmc_host *mmc,
>> if (host->ops->voltage_switch)
>> host->ops->voltage_switch(host);
>>
>> + /* Wait for 5ms */
>> + usleep_range(5000, 5500);
> We have managed without this delay for a while and I am reluctant to add
> delays that might anyway be specific to the controller. What do you think
> about implementing ->voltage_switch() and putting the delay there?
Ok to me, will send out the updated version. Thanks.
>
>> +
>> /* 1.8V regulator output should be stable within 5 ms */
>> ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);
>> if (ctrl & SDHCI_CTRL_VDD_180)
>>
^ permalink raw reply
* Re: [RFC PATCH] mmc: tmio: use ioread* for repeated access to a register
From: Masahiro Yamada @ 2017-12-18 16:01 UTC (permalink / raw)
To: Ulf Hansson, Wolfram Sang
Cc: linux-mmc@vger.kernel.org, Linux-Renesas, Simon Horman
In-Reply-To: <CAPDyKFr5A7Qma_Ef5JMjvG1f_Er+f9K-tnj39_q0Wg2aQ7nY5w@mail.gmail.com>
2017-12-18 22:01 GMT+09:00 Ulf Hansson <ulf.hansson@linaro.org>:
> On 18 December 2017 at 01:00, Wolfram Sang
> <wsa+renesas@sang-engineering.com> wrote:
>> Not all archs define reads* and writes*. Switch to ioread*_rep and
>> friends which is defined everywhere, so we can enable COMPILE_TEST after
>> that.
>>
>> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
>
> Thanks, applied for next!
>
> I put the patch in front of Yamada-san's patch that made it possible
> to use COMPILE_TEST. Thus avoiding breaking bisecting.
>
>> ---
>>
>> So, I pushed this to buildbot on top of Yamada-san's patch series and there
>> were no complaints, even with COMPILE_TEST enabled. I also did some tests on
>> HW and checksuming huge files on SD cards still works.
>>
>> However, I am not sure about this mixture of read* and ioread* functions. Shall
>> we convert maybe all of those?
>
> Yes, seems like a good idea.
>
> [...]
>
BTW, is ioread* preferred to read*?
We need to eliminate the root cause.
Other drivers using reads* cannot enable COMPILE_TEST for the same reason.
Wolfram,
Can you send a patch for sparc64?
--
Best Regards
Masahiro Yamada
^ permalink raw reply
* Re: [PATCH v3] mmc: sdhci: wait 5ms after set 1.8V signal enable
From: Adrian Hunter @ 2017-12-18 13:19 UTC (permalink / raw)
To: Zhoujie Wu, ulf.hansson, linux-mmc
Cc: nadavh, xigu, xswang, dingwei, kostap, hannah, hongd, dougj, ygao,
liuw, gregory.clement, thomas.petazzoni
In-Reply-To: <1513366136-20302-1-git-send-email-zjwu@marvell.com>
On 15/12/17 21:28, Zhoujie Wu wrote:
> According to SD spec 3.00 3.6.1 signal voltage switch
> procedure step 6~8,
> (6) Set 1.8V Signal Enable in the Host Control 2 register.
> (7) Wait 5ms. 1.8V voltage regulator shall be stable within this period.
> (8) If 1.8V Signal Enable is cleared by Host Controller, go to step (12).
>
> Host should wait 5ms after set 1.8V signal enable bit in
> Host Control 2 register and check if 1.8V is stable or not.
>
> But current code checks if this bit is cleared by controller
> right after set it. On some platforms found the bit is not
> cleared right away and host reports "1.8V regulator output
> did not became stable" and 5ms delay can help.
The check is for the bit set not cleared.
>
> Follow the spec and add 5ms delay to make sure the 1.8V Signal Enable
> bit is cleared.
>
> Signed-off-by: Zhoujie Wu <zjwu@marvell.com>
> ---
> drivers/mmc/host/sdhci.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index e9290a3..fe5f208 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -1933,6 +1933,9 @@ int sdhci_start_signal_voltage_switch(struct mmc_host *mmc,
> if (host->ops->voltage_switch)
> host->ops->voltage_switch(host);
>
> + /* Wait for 5ms */
> + usleep_range(5000, 5500);
We have managed without this delay for a while and I am reluctant to add
delays that might anyway be specific to the controller. What do you think
about implementing ->voltage_switch() and putting the delay there?
> +
> /* 1.8V regulator output should be stable within 5 ms */
> ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);
> if (ctrl & SDHCI_CTRL_VDD_180)
>
^ permalink raw reply
* Re: [PATCH v2] mmc: block: fix logical error to avoid memory leak
From: Liu, Changcheng @ 2017-12-18 13:16 UTC (permalink / raw)
To: Ulf Hansson, adrian.hunter; +Cc: linus.walleij, linux-mmc, akpm
In-Reply-To: <CAPDyKFojyPB5QULubDVar1kqnHQ0SUeZn7kr=HWKfDiWmujQ6Q@mail.gmail.com>
Hi Uffe,
Thx for your guide. I'm on the way to work with community. I'll
follow your suggestion next time.
B.R.
Changcheng
On 21:05 Mon 18 Dec, Ulf Hansson wrote:
> On 16 December 2017 at 16:15, Liu, Changcheng <changcheng.liu@intel.com> wrote:
> > If the MMC_DRV_OP_GET_EXT_CSD request completes successfully, then
> > ext_csd must be freed, but in one case it was not. Fix that.
> >
> > Signed-off-by: Liu Changcheng <changcheng.liu@intel.com>
>
> Thanks, applied for next!
>
> Next time, you may please preserve acks from previous versions,
> especially if the reviewer (Adrian in this case) says so.
>
> Kind regards
> Uffe
>
> >
> > diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
> > index ccfa98a..b737a95 100644
> > --- a/drivers/mmc/core/block.c
> > +++ b/drivers/mmc/core/block.c
> > @@ -2623,6 +2623,7 @@ static int mmc_ext_csd_open(struct inode *inode, struct file *filp)
> >
> > if (n != EXT_CSD_STR_LEN) {
> > err = -EINVAL;
> > + kfree(ext_csd);
> > goto out_free;
> > }
> >
> > --
> > 2.7.4
^ permalink raw reply
* Re: [PATCH v2] mmc: block: fix logical error to avoid memory leak
From: Ulf Hansson @ 2017-12-18 13:05 UTC (permalink / raw)
To: Liu, Changcheng
Cc: Adrian Hunter, Linus Walleij, linux-mmc@vger.kernel.org,
Andrew Morton
In-Reply-To: <20171216151545.GA138678@sofia>
On 16 December 2017 at 16:15, Liu, Changcheng <changcheng.liu@intel.com> wrote:
> If the MMC_DRV_OP_GET_EXT_CSD request completes successfully, then
> ext_csd must be freed, but in one case it was not. Fix that.
>
> Signed-off-by: Liu Changcheng <changcheng.liu@intel.com>
Thanks, applied for next!
Next time, you may please preserve acks from previous versions,
especially if the reviewer (Adrian in this case) says so.
Kind regards
Uffe
>
> diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
> index ccfa98a..b737a95 100644
> --- a/drivers/mmc/core/block.c
> +++ b/drivers/mmc/core/block.c
> @@ -2623,6 +2623,7 @@ static int mmc_ext_csd_open(struct inode *inode, struct file *filp)
>
> if (n != EXT_CSD_STR_LEN) {
> err = -EINVAL;
> + kfree(ext_csd);
> goto out_free;
> }
>
> --
> 2.7.4
^ permalink raw reply
* Re: [RFC PATCH] mmc: tmio: use ioread* for repeated access to a register
From: Ulf Hansson @ 2017-12-18 13:01 UTC (permalink / raw)
To: Wolfram Sang
Cc: linux-mmc@vger.kernel.org, Linux-Renesas, Masahiro Yamada,
Simon Horman
In-Reply-To: <20171218000021.16655-1-wsa+renesas@sang-engineering.com>
On 18 December 2017 at 01:00, Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
> Not all archs define reads* and writes*. Switch to ioread*_rep and
> friends which is defined everywhere, so we can enable COMPILE_TEST after
> that.
>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Thanks, applied for next!
I put the patch in front of Yamada-san's patch that made it possible
to use COMPILE_TEST. Thus avoiding breaking bisecting.
> ---
>
> So, I pushed this to buildbot on top of Yamada-san's patch series and there
> were no complaints, even with COMPILE_TEST enabled. I also did some tests on
> HW and checksuming huge files on SD cards still works.
>
> However, I am not sure about this mixture of read* and ioread* functions. Shall
> we convert maybe all of those?
Yes, seems like a good idea.
[...]
Kind regards
Uffe
^ permalink raw reply
* [ulf.hansson-mmc:next 51/60] drivers/mmc/host/tmio_mmc.h:249:2: error: implicit declaration of function 'readsw'; did you mean 'readw'?
From: kbuild test robot @ 2017-12-18 5:55 UTC (permalink / raw)
To: Masahiro Yamada; +Cc: kbuild-all, linux-mmc, Ulf Hansson, Geert Uytterhoeven
[-- Attachment #1: Type: text/plain, Size: 6700 bytes --]
tree: git://git.kernel.org/pub/scm/linux/kernel/git/ulfh/mmc.git next
head: aebb32cecdd48833ca6bb1f360df6f9039b55e56
commit: f8f556bf652a57d5e126680d92690ecef8e75f2d [51/60] mmc: renesas_sdhi: remove wrong depends on to enable compile test
config: m32r-allyesconfig (attached as .config)
compiler: m32r-linux-gcc (GCC) 7.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
git checkout f8f556bf652a57d5e126680d92690ecef8e75f2d
# save the attached .config to linux build tree
make.cross ARCH=m32r
All errors (new ones prefixed by >>):
In file included from arch/m32r/include/uapi/asm/byteorder.h:8:0,
from arch/m32r/include/asm/bitops.h:22,
from include/linux/bitops.h:38,
from include/linux/kernel.h:11,
from include/linux/delay.h:22,
from drivers/mmc/host/tmio_mmc_core.c:30:
include/linux/byteorder/big_endian.h:8:2: warning: #warning inconsistent configuration, needs CONFIG_CPU_BIG_ENDIAN [-Wcpp]
#warning inconsistent configuration, needs CONFIG_CPU_BIG_ENDIAN
^~~~~~~
In file included from drivers/mmc/host/tmio_mmc_core.c:53:0:
drivers/mmc/host/tmio_mmc.h: In function 'sd_ctrl_read16_rep':
>> drivers/mmc/host/tmio_mmc.h:249:2: error: implicit declaration of function 'readsw'; did you mean 'readw'? [-Werror=implicit-function-declaration]
readsw(host->ctl + (addr << host->bus_shift), buf, count);
^~~~~~
readw
drivers/mmc/host/tmio_mmc.h: In function 'sd_ctrl_read32_rep':
>> drivers/mmc/host/tmio_mmc.h:262:2: error: implicit declaration of function 'readsl'; did you mean 'readl'? [-Werror=implicit-function-declaration]
readsl(host->ctl + (addr << host->bus_shift), buf, count);
^~~~~~
readl
drivers/mmc/host/tmio_mmc.h: In function 'sd_ctrl_write16_rep':
>> drivers/mmc/host/tmio_mmc.h:279:2: error: implicit declaration of function 'writesw'; did you mean 'writew'? [-Werror=implicit-function-declaration]
writesw(host->ctl + (addr << host->bus_shift), buf, count);
^~~~~~~
writew
drivers/mmc/host/tmio_mmc.h: In function 'sd_ctrl_write32_rep':
>> drivers/mmc/host/tmio_mmc.h:292:2: error: implicit declaration of function 'writesl'; did you mean 'writel'? [-Werror=implicit-function-declaration]
writesl(host->ctl + (addr << host->bus_shift), buf, count);
^~~~~~~
writel
cc1: some warnings being treated as errors
vim +249 drivers/mmc/host/tmio_mmc.h
a11862d3 Simon Horman 2011-06-21 245
a11862d3 Simon Horman 2011-06-21 246 static inline void sd_ctrl_read16_rep(struct tmio_mmc_host *host, int addr,
a11862d3 Simon Horman 2011-06-21 247 u16 *buf, int count)
a11862d3 Simon Horman 2011-06-21 248 {
7445bf9e Kuninori Morimoto 2015-01-13 @249 readsw(host->ctl + (addr << host->bus_shift), buf, count);
a11862d3 Simon Horman 2011-06-21 250 }
a11862d3 Simon Horman 2011-06-21 251
f2218db8 Simon Horman 2017-06-16 252 static inline u32 sd_ctrl_read16_and_16_as_32(struct tmio_mmc_host *host,
f2218db8 Simon Horman 2017-06-16 253 int addr)
a11862d3 Simon Horman 2011-06-21 254 {
7445bf9e Kuninori Morimoto 2015-01-13 255 return readw(host->ctl + (addr << host->bus_shift)) |
7445bf9e Kuninori Morimoto 2015-01-13 256 readw(host->ctl + ((addr + 2) << host->bus_shift)) << 16;
a11862d3 Simon Horman 2011-06-21 257 }
a11862d3 Simon Horman 2011-06-21 258
8185e51f Chris Brandt 2016-09-12 259 static inline void sd_ctrl_read32_rep(struct tmio_mmc_host *host, int addr,
8185e51f Chris Brandt 2016-09-12 260 u32 *buf, int count)
8185e51f Chris Brandt 2016-09-12 261 {
8185e51f Chris Brandt 2016-09-12 @262 readsl(host->ctl + (addr << host->bus_shift), buf, count);
8185e51f Chris Brandt 2016-09-12 263 }
8185e51f Chris Brandt 2016-09-12 264
f2218db8 Simon Horman 2017-06-16 265 static inline void sd_ctrl_write16(struct tmio_mmc_host *host, int addr,
f2218db8 Simon Horman 2017-06-16 266 u16 val)
a11862d3 Simon Horman 2011-06-21 267 {
973ed3af Simon Horman 2011-06-21 268 /* If there is a hook and it returns non-zero then there
973ed3af Simon Horman 2011-06-21 269 * is an error and the write should be skipped
973ed3af Simon Horman 2011-06-21 270 */
dfe9a229 Kuninori Morimoto 2015-01-13 271 if (host->write16_hook && host->write16_hook(host, addr))
973ed3af Simon Horman 2011-06-21 272 return;
7445bf9e Kuninori Morimoto 2015-01-13 273 writew(val, host->ctl + (addr << host->bus_shift));
a11862d3 Simon Horman 2011-06-21 274 }
a11862d3 Simon Horman 2011-06-21 275
a11862d3 Simon Horman 2011-06-21 276 static inline void sd_ctrl_write16_rep(struct tmio_mmc_host *host, int addr,
a11862d3 Simon Horman 2011-06-21 277 u16 *buf, int count)
a11862d3 Simon Horman 2011-06-21 278 {
7445bf9e Kuninori Morimoto 2015-01-13 @279 writesw(host->ctl + (addr << host->bus_shift), buf, count);
a11862d3 Simon Horman 2011-06-21 280 }
a11862d3 Simon Horman 2011-06-21 281
f2218db8 Simon Horman 2017-06-16 282 static inline void sd_ctrl_write32_as_16_and_16(struct tmio_mmc_host *host,
f2218db8 Simon Horman 2017-06-16 283 int addr, u32 val)
a11862d3 Simon Horman 2011-06-21 284 {
7c42dbf3 Wolfram Sang 2016-05-27 285 writew(val & 0xffff, host->ctl + (addr << host->bus_shift));
7445bf9e Kuninori Morimoto 2015-01-13 286 writew(val >> 16, host->ctl + ((addr + 2) << host->bus_shift));
a11862d3 Simon Horman 2011-06-21 287 }
a11862d3 Simon Horman 2011-06-21 288
8185e51f Chris Brandt 2016-09-12 289 static inline void sd_ctrl_write32_rep(struct tmio_mmc_host *host, int addr,
8185e51f Chris Brandt 2016-09-12 290 const u32 *buf, int count)
8185e51f Chris Brandt 2016-09-12 291 {
8185e51f Chris Brandt 2016-09-12 @292 writesl(host->ctl + (addr << host->bus_shift), buf, count);
8185e51f Chris Brandt 2016-09-12 293 }
8185e51f Chris Brandt 2016-09-12 294
:::::: The code at line 249 was first introduced by commit
:::::: 7445bf9e6f4e5d7755e22c7c9b06f4ae0d6160c6 mmc: tmio: tmio_mmc_host has .bus_shift
:::::: TO: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
:::::: CC: Ulf Hansson <ulf.hansson@linaro.org>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 44613 bytes --]
^ permalink raw reply
* Re: [RFC PATCH] mmc: tmio: use ioread* for repeated access to a register
From: Ian Molton @ 2017-12-18 0:13 UTC (permalink / raw)
To: Wolfram Sang, linux-mmc; +Cc: linux-renesas-soc, Masahiro Yamada, Simon Horman
In-Reply-To: <20171218000021.16655-1-wsa+renesas@sang-engineering.com>
On 18/12/17 00:00, Wolfram Sang wrote:
> Not all archs define reads* and writes*. Switch to ioread*_rep and
> friends which is defined everywhere, so we can enable COMPILE_TEST after
> that.
>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> ---
>
> So, I pushed this to buildbot on top of Yamada-san's patch series and there
> were no complaints, even with COMPILE_TEST enabled. I also did some tests on
> HW and checksuming huge files on SD cards still works.
>
> However, I am not sure about this mixture of read* and ioread* functions. Shall
> we convert maybe all of those?
Might as well. They've been in Linux since what 2.6.9?
> drivers/mmc/host/tmio_mmc.h | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
> index 76094345cbacf3..03519c4ca0aa1a 100644
> --- a/drivers/mmc/host/tmio_mmc.h
> +++ b/drivers/mmc/host/tmio_mmc.h
> @@ -233,7 +233,7 @@ static inline u16 sd_ctrl_read16(struct tmio_mmc_host *host, int addr)
> static inline void sd_ctrl_read16_rep(struct tmio_mmc_host *host, int addr,
> u16 *buf, int count)
> {
> - readsw(host->ctl + (addr << host->bus_shift), buf, count);
> + ioread16_rep(host->ctl + (addr << host->bus_shift), buf, count);
> }
>
> static inline u32 sd_ctrl_read16_and_16_as_32(struct tmio_mmc_host *host,
> @@ -246,7 +246,7 @@ static inline u32 sd_ctrl_read16_and_16_as_32(struct tmio_mmc_host *host,
> static inline void sd_ctrl_read32_rep(struct tmio_mmc_host *host, int addr,
> u32 *buf, int count)
> {
> - readsl(host->ctl + (addr << host->bus_shift), buf, count);
> + ioread32_rep(host->ctl + (addr << host->bus_shift), buf, count);
> }
>
> static inline void sd_ctrl_write16(struct tmio_mmc_host *host, int addr,
> @@ -263,7 +263,7 @@ static inline void sd_ctrl_write16(struct tmio_mmc_host *host, int addr,
> static inline void sd_ctrl_write16_rep(struct tmio_mmc_host *host, int addr,
> u16 *buf, int count)
> {
> - writesw(host->ctl + (addr << host->bus_shift), buf, count);
> + iowrite16_rep(host->ctl + (addr << host->bus_shift), buf, count);
> }
>
> static inline void sd_ctrl_write32_as_16_and_16(struct tmio_mmc_host *host,
> @@ -276,7 +276,7 @@ static inline void sd_ctrl_write32_as_16_and_16(struct tmio_mmc_host *host,
> static inline void sd_ctrl_write32_rep(struct tmio_mmc_host *host, int addr,
> const u32 *buf, int count)
> {
> - writesl(host->ctl + (addr << host->bus_shift), buf, count);
> + iowrite32_rep(host->ctl + (addr << host->bus_shift), buf, count);
> }
>
> #endif
>
^ permalink raw reply
* [RFC PATCH] mmc: tmio: use ioread* for repeated access to a register
From: Wolfram Sang @ 2017-12-18 0:00 UTC (permalink / raw)
To: linux-mmc; +Cc: linux-renesas-soc, Masahiro Yamada, Simon Horman, Wolfram Sang
Not all archs define reads* and writes*. Switch to ioread*_rep and
friends which is defined everywhere, so we can enable COMPILE_TEST after
that.
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
So, I pushed this to buildbot on top of Yamada-san's patch series and there
were no complaints, even with COMPILE_TEST enabled. I also did some tests on
HW and checksuming huge files on SD cards still works.
However, I am not sure about this mixture of read* and ioread* functions. Shall
we convert maybe all of those?
drivers/mmc/host/tmio_mmc.h | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index 76094345cbacf3..03519c4ca0aa1a 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -233,7 +233,7 @@ static inline u16 sd_ctrl_read16(struct tmio_mmc_host *host, int addr)
static inline void sd_ctrl_read16_rep(struct tmio_mmc_host *host, int addr,
u16 *buf, int count)
{
- readsw(host->ctl + (addr << host->bus_shift), buf, count);
+ ioread16_rep(host->ctl + (addr << host->bus_shift), buf, count);
}
static inline u32 sd_ctrl_read16_and_16_as_32(struct tmio_mmc_host *host,
@@ -246,7 +246,7 @@ static inline u32 sd_ctrl_read16_and_16_as_32(struct tmio_mmc_host *host,
static inline void sd_ctrl_read32_rep(struct tmio_mmc_host *host, int addr,
u32 *buf, int count)
{
- readsl(host->ctl + (addr << host->bus_shift), buf, count);
+ ioread32_rep(host->ctl + (addr << host->bus_shift), buf, count);
}
static inline void sd_ctrl_write16(struct tmio_mmc_host *host, int addr,
@@ -263,7 +263,7 @@ static inline void sd_ctrl_write16(struct tmio_mmc_host *host, int addr,
static inline void sd_ctrl_write16_rep(struct tmio_mmc_host *host, int addr,
u16 *buf, int count)
{
- writesw(host->ctl + (addr << host->bus_shift), buf, count);
+ iowrite16_rep(host->ctl + (addr << host->bus_shift), buf, count);
}
static inline void sd_ctrl_write32_as_16_and_16(struct tmio_mmc_host *host,
@@ -276,7 +276,7 @@ static inline void sd_ctrl_write32_as_16_and_16(struct tmio_mmc_host *host,
static inline void sd_ctrl_write32_rep(struct tmio_mmc_host *host, int addr,
const u32 *buf, int count)
{
- writesl(host->ctl + (addr << host->bus_shift), buf, count);
+ iowrite32_rep(host->ctl + (addr << host->bus_shift), buf, count);
}
#endif
--
2.11.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox