Re: [RFC PATCH v4 1/9] mmc: dw_mmc: Add external dma interface support
在 2015/8/8 5:32, Joachim Eastwood 写道:
Hi Shawn,
On 6 August 2015 at 08:44, Shawn Lin wrote:
DesignWare MMC Controller can supports two types of DMA
mode: external dma and internal dma. We get a RK312x platform
integrated dw_mmc and ARM pl330 dma controller. This patch add
edmac ops to support these platforms. I've tested it on RK312x
platform with edmac mode and RK3288 platform with idmac mode.
Signed-off-by: Shawn Lin
@@ -2256,26 +2373,30 @@ static irqreturn_t dw_mci_interrupt(int irq, void
*dev_id)
}
-#ifdef CONFIG_MMC_DW_IDMAC
- /* Handle DMA interrupts */
- if (host->dma_64bit_address == 1) {
- pending = mci_readl(host, IDSTS64);
- if (pending & (SDMMC_IDMAC_INT_TI | SDMMC_IDMAC_INT_RI)) {
- mci_writel(host, IDSTS64, SDMMC_IDMAC_INT_TI |
- SDMMC_IDMAC_INT_RI);
- mci_writel(host, IDSTS64, SDMMC_IDMAC_INT_NI);
- host->dma_ops->complete(host);
- }
- } else {
- pending = mci_readl(host, IDSTS);
- if (pending & (SDMMC_IDMAC_INT_TI | SDMMC_IDMAC_INT_RI)) {
- mci_writel(host, IDSTS, SDMMC_IDMAC_INT_TI |
- SDMMC_IDMAC_INT_RI);
- mci_writel(host, IDSTS, SDMMC_IDMAC_INT_NI);
- host->dma_ops->complete(host);
+ if (host->use_dma == TRANS_MODE_IDMAC) {
Doing:
if (host->use_dma != TRANS_MODE_IDMAC)
return IRQ_HANDLED;
Okay.
Could save you the extra level of identation you add below.
+ /* Handle DMA interrupts */
+ if (host->dma_64bit_address == 1) {
+ pending = mci_readl(host, IDSTS64);
+ if (pending & (SDMMC_IDMAC_INT_TI |
+ SDMMC_IDMAC_INT_RI)) {
+ mci_writel(host, IDSTS64,
+ SDMMC_IDMAC_INT_TI |
+ SDMMC_IDMAC_INT_RI);
+ mci_writel(host, IDSTS64, SDMMC_IDMAC_INT_NI);
+ host->dma_ops->complete((void *)host);
+ }
+ } else {
+ pending = mci_readl(host, IDSTS);
+ if (pending & (SDMMC_IDMAC_INT_TI |
+ SDMMC_IDMAC_INT_RI)) {
+ mci_writel(host, IDSTS,
+ SDMMC_IDMAC_INT_TI |
+ SDMMC_IDMAC_INT_RI);
+ mci_writel(host, IDSTS, SDMMC_IDMAC_INT_NI);
+ host->dma_ops->complete((void *)host);
+ }
}
}
-#endif
return IRQ_HANDLED;
}
@@ -2437,6 +2567,21 @@ static void dw_mci_cleanup_slot(struct dw_mci_slot
*slot, unsigned int id)
static void dw_mci_init_dma(struct dw_mci *host)
{
int addr_config;
+ int trans_mode;
+ struct device *dev = host->dev;
+ struct device_node *np = dev->of_node;
+
+ /* Check tansfer mode */
+ trans_mode = (mci_readl(host, HCON) >> 16) & 0x3;
I think it would be nice if you could add some defines for 16 and 0x03
or add a macro like SDMMC_GET_FCNT() that is in dw_mmc.h.
yes, it's better to avoid magic number for register operation to make
others understand w/o checking databook for details. And might more than
one (e.g "Check ADDR_CONFIG bit in HCON to find IDMAC address bus
width") should be modified.
Although one patch only do one thing, I will drop by to make it in v5.
+ if (trans_mode == 0) {
+ trans_mode = TRANS_MODE_IDMAC;
+ } else if (trans_mode == 1 || trans_mode == 2) {
+ trans_mode = TRANS_MODE_EDMAC;
+ } else {
+ trans_mode = TRANS_MODE_PIO;
+ goto no_dma;
+ }
+
/* Check ADDR_CONFIG bit in HCON to find IDMAC address bus width */
addr_config = (mci_readl(host, HCON) >> 27) & 0x01;
I'll try to get this patch tested on my lpc18xx platform soon.
btw, the HCON reg on lpc18xx reads as 0x00e42cc1 (address 0x40004070).
yes, HCON[17:16] is 2b'00 means your lpc18xx use IDMAC.
regard,
Joachim Eastwood
--
Shawn Lin
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH v4 7/9] arm: lpc18xx_defconfig: remove CONFIG_MMC_DW_IDMAC
On 6 August 2015 at 08:46, Shawn Lin wrote: > DesignWare MMC Controller's transfer mode should be decided > at runtime instead of compile-time. So we remove this config > option and read dw_mmc's register to select DMA master. > > Signed-off-by: Shawn Lin Acked-by: Joachim Eastwood > arch/arm/configs/lpc18xx_defconfig | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/arch/arm/configs/lpc18xx_defconfig > b/arch/arm/configs/lpc18xx_defconfig > index 1c47f86..b7e8cda 100644 > --- a/arch/arm/configs/lpc18xx_defconfig > +++ b/arch/arm/configs/lpc18xx_defconfig > @@ -119,7 +119,6 @@ CONFIG_USB_EHCI_HCD=y > CONFIG_USB_EHCI_ROOT_HUB_TT=y > CONFIG_MMC=y > CONFIG_MMC_DW=y > -CONFIG_MMC_DW_IDMAC=y > CONFIG_NEW_LEDS=y > CONFIG_LEDS_CLASS=y > CONFIG_LEDS_PCA9532=y > -- > 2.3.7 > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH v4 1/9] mmc: dw_mmc: Add external dma interface support
Hi Shawn,
On 6 August 2015 at 08:44, Shawn Lin wrote:
> DesignWare MMC Controller can supports two types of DMA
> mode: external dma and internal dma. We get a RK312x platform
> integrated dw_mmc and ARM pl330 dma controller. This patch add
> edmac ops to support these platforms. I've tested it on RK312x
> platform with edmac mode and RK3288 platform with idmac mode.
>
> Signed-off-by: Shawn Lin
> @@ -2256,26 +2373,30 @@ static irqreturn_t dw_mci_interrupt(int irq, void
> *dev_id)
>
> }
>
> -#ifdef CONFIG_MMC_DW_IDMAC
> - /* Handle DMA interrupts */
> - if (host->dma_64bit_address == 1) {
> - pending = mci_readl(host, IDSTS64);
> - if (pending & (SDMMC_IDMAC_INT_TI | SDMMC_IDMAC_INT_RI)) {
> - mci_writel(host, IDSTS64, SDMMC_IDMAC_INT_TI |
> - SDMMC_IDMAC_INT_RI);
> - mci_writel(host, IDSTS64, SDMMC_IDMAC_INT_NI);
> - host->dma_ops->complete(host);
> - }
> - } else {
> - pending = mci_readl(host, IDSTS);
> - if (pending & (SDMMC_IDMAC_INT_TI | SDMMC_IDMAC_INT_RI)) {
> - mci_writel(host, IDSTS, SDMMC_IDMAC_INT_TI |
> - SDMMC_IDMAC_INT_RI);
> - mci_writel(host, IDSTS, SDMMC_IDMAC_INT_NI);
> - host->dma_ops->complete(host);
> + if (host->use_dma == TRANS_MODE_IDMAC) {
Doing:
if (host->use_dma != TRANS_MODE_IDMAC)
return IRQ_HANDLED;
Could save you the extra level of identation you add below.
> + /* Handle DMA interrupts */
> + if (host->dma_64bit_address == 1) {
> + pending = mci_readl(host, IDSTS64);
> + if (pending & (SDMMC_IDMAC_INT_TI |
> + SDMMC_IDMAC_INT_RI)) {
> + mci_writel(host, IDSTS64,
> + SDMMC_IDMAC_INT_TI |
> + SDMMC_IDMAC_INT_RI);
> + mci_writel(host, IDSTS64, SDMMC_IDMAC_INT_NI);
> + host->dma_ops->complete((void *)host);
> + }
> + } else {
> + pending = mci_readl(host, IDSTS);
> + if (pending & (SDMMC_IDMAC_INT_TI |
> + SDMMC_IDMAC_INT_RI)) {
> + mci_writel(host, IDSTS,
> + SDMMC_IDMAC_INT_TI |
> + SDMMC_IDMAC_INT_RI);
> + mci_writel(host, IDSTS, SDMMC_IDMAC_INT_NI);
> + host->dma_ops->complete((void *)host);
> + }
> }
> }
> -#endif
>
> return IRQ_HANDLED;
> }
> @@ -2437,6 +2567,21 @@ static void dw_mci_cleanup_slot(struct dw_mci_slot
> *slot, unsigned int id)
> static void dw_mci_init_dma(struct dw_mci *host)
> {
> int addr_config;
> + int trans_mode;
> + struct device *dev = host->dev;
> + struct device_node *np = dev->of_node;
> +
> + /* Check tansfer mode */
> + trans_mode = (mci_readl(host, HCON) >> 16) & 0x3;
I think it would be nice if you could add some defines for 16 and 0x03
or add a macro like SDMMC_GET_FCNT() that is in dw_mmc.h.
> + if (trans_mode == 0) {
> + trans_mode = TRANS_MODE_IDMAC;
> + } else if (trans_mode == 1 || trans_mode == 2) {
> + trans_mode = TRANS_MODE_EDMAC;
> + } else {
> + trans_mode = TRANS_MODE_PIO;
> + goto no_dma;
> + }
> +
> /* Check ADDR_CONFIG bit in HCON to find IDMAC address bus width */
> addr_config = (mci_readl(host, HCON) >> 27) & 0x01;
I'll try to get this patch tested on my lpc18xx platform soon.
btw, the HCON reg on lpc18xx reads as 0x00e42cc1 (address 0x40004070).
regard,
Joachim Eastwood
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/11] omap_hsmmc: voltage switching and tuning
Hi, On Thursday 06 August 2015 12:18 PM, Tony Lindgren wrote: > * Kishon Vijay Abraham I [150805 08:03]: >> Hi, >> >> On Wednesday 05 August 2015 04:13 PM, Tony Lindgren wrote: >>> * Kishon Vijay Abraham I [150730 00:49]: Patch series implements voltage switching and tuning for omap_hsmmc driver. Did basic read/write test in J6, J6 Eco, Beagle-x15, AM437x EVM, Beaglebone black, OMAP5 uEVM and OMAP4 PANDA. >>> >>> Your tests are missing omap3? >> >> I don't have one at my disposal :-( I'll try to find one and add omap3 tests. > > Great :) Beagle xm is a good one to test the USB PHY stuff on > and also MMC. Having USB cable connected or EHCI loaded blocks the PM > though, so NFSroot is not very usable for testing on it. I found a beagle xm. So I should be able to get back to this early next week. Thanks Kishon > > If you want to test PM over NFSroot, boards with the smsc911x known > to work with PM are at least omap3-evm-37xx.dts and > logicpd-torpedo-37xx-devkit.dts. Since you tinker with the USB PHYs, > torpedo has MUSB working the same way as the beagle boards, EVM I > think can be modified that way but by default has diffent PHY. > > Also overo tobi boards work with PM but only for retntion idle and > not off idle, and map3-sbc-t3517.dts probably can be made to work > with PM but I have mine as a gateway and have not been able to test > with it. > > Probably also zoom3 boards with later processor boards can be made > to work, the LDP has early omap34xx variants and can't be made to > work reliably. > > I don't have omap3-lilly, but I'd assume that can also be made > to work with PM if not already working. > >>> I suggest you add some omap3 tests in general as otherwise you are >>> only testing a subset of the driver features and completely missing >>> things like rutnime PM and save and restore for the deeper idle >>> states. >> >> yeah, I'll do those tests and re-post the series. > > Thanks! > > Tony > -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC PATCH] ARM: dts: rockchip: Add wifi support for firefly
This patch enable wifi support for the firefly board.
Card answer to me that support from 2.0V but regulator is connected
to 1.8V, so voltage capability is wrong. In order to avoid this
we just defined a fixed regulator trigger by the wifi enable gpio
that report 2.0V.
Signed-off-by: Michael Trimarchi
---
arch/arm/boot/dts/rk3288-firefly.dtsi | 43 +--
1 file changed, 41 insertions(+), 2 deletions(-)
diff --git a/arch/arm/boot/dts/rk3288-firefly.dtsi
b/arch/arm/boot/dts/rk3288-firefly.dtsi
index 0b42372..fcf234e 100644
--- a/arch/arm/boot/dts/rk3288-firefly.dtsi
+++ b/arch/arm/boot/dts/rk3288-firefly.dtsi
@@ -116,6 +116,28 @@
vin-supply = <&vcc_io>;
};
+ io_domains: io-domains {
+ compatible = "rockchip,rk3288-io-voltage-domain";
+ rockchip,grf = <&grf>;
+
+ wifi-supply = <&vcc_18>;
+ audio-supply = <&vcca_33>;
+ };
+
+ vcc_wifi: wifi-regulator {
+ compatible = "regulator-fixed";
+ enable-active-high;
+ gpio = <&gpio4 28 GPIO_ACTIVE_HIGH>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&sdio0_pwr>;
+ regulator-name = "vcc_wifi";
+ regulator-min-microvolt = <200>;
+ regulator-max-microvolt = <200>;
+ startup-delay-us = <10>;
+ regulator-always-on;
+ vin-supply = <&vcc_io>;
+ };
+
vcc_flash: flash-regulator {
compatible = "regulator-fixed";
regulator-name = "vcc_flash";
@@ -437,13 +459,30 @@
&sdio0 {
broken-cd;
bus-width = <4>;
+ clock-freq-min-max = <40 5000>;
disable-wp;
non-removable;
+ cap-sd-highspeed;
num-slots = <1>;
pinctrl-names = "default";
- pinctrl-0 = <&sdio0_bus4>, <&sdio0_cmd>, <&sdio0_clk>;
- vmmc-supply = <&vcc_18>;
+ pinctrl-0 = <&sdio0_bus4>, <&sdio0_cmd>, <&sdio0_clk>, <&sdio0_int>;
+
+ sd-uhs-sdr50;
+ sd-uhs-sdr104;
+ sd-uhs-ddr50;
+ cap-sdio-irq;
+
+ vmmc-supply = <&vcc_wifi>;
status = "okay";
+
+ brcmf: bcrmf@0 {
+ compatible = "brcm,bcm4329-fmac";
+ interrupt-parent = <&gpio4>;
+ reg = <0>;
+ interrupts = <30 IRQ_TYPE_EDGE_FALLING>;
+ interrupt-names = "host-wake";
+ status = "okay";
+ };
};
&sdmmc {
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 1/6] mmc: sdhci-esdhc-imx: add imx7d support and support HS400
On Fri, Aug 07, 2015 at 05:53:01PM +0800, Chen Haibo-B51421 wrote:
>
>
> > -Original Message-
> > From: Dong Aisheng [mailto:aisheng.d...@freescale.com]
> > Sent: Friday, August 07, 2015 3:39 PM
> > To: Chen Haibo-B51421
> > Cc: robh...@kernel.org; pawel.m...@arm.com; mark.rutl...@arm.com;
> > ijc+devicet...@hellion.org.uk; ga...@codeaurora.org; shawn...@kernel.org;
> > ker...@pengutronix.de; li...@arm.linux.org.uk; ulf.hans...@linaro.org;
> > johan.dery...@barco.com; m...@pengutronix.de; Estevam Fabio-R49496; Dong
> > Aisheng-B29396; devicet...@vger.kernel.org; linux-ker...@vger.kernel.org;
> > linux-arm-ker...@lists.infradead.org; linux-mmc@vger.kernel.org
> > Subject: Re: [PATCH v4 1/6] mmc: sdhci-esdhc-imx: add imx7d support and
> > support HS400
> >
> > On Wed, Aug 05, 2015 at 06:38:37PM +0800, Haibo Chen wrote:
> > > The imx7d usdhc is derived from imx6sx, the difference is that imx7d
> > > support HS400.
> > >
> > > So introduce a new compatible string for imx7d and add HS400 support
> > > for imx7d usdhc.
> > >
> > > Signed-off-by: Haibo Chen
> > > ---
> > > drivers/mmc/host/sdhci-esdhc-imx.c | 81
> > > ++
> > > 1 file changed, 81 insertions(+)
> > >
> > > diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c
> > > b/drivers/mmc/host/sdhci-esdhc-imx.c
> > > index c6b9f64..48f009c 100644
> > > --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> > > +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> > > @@ -44,6 +44,7 @@
> > > #define ESDHC_MIX_CTRL_EXE_TUNE (1 << 22)
> > > #define ESDHC_MIX_CTRL_SMPCLK_SEL (1 << 23)
> > > #define ESDHC_MIX_CTRL_FBCLK_SEL(1 << 25)
> > > +#define ESDHC_MIX_CTRL_HS400_EN (1 << 26)
> > > /* Bits 3 and 6 are not SDHCI standard definitions */
> > > #define ESDHC_MIX_CTRL_SDHCI_MASK 0xb7
> > > /* Tuning bits */
> > > @@ -60,6 +61,16 @@
> > > #define ESDHC_TUNE_CTRL_MIN 0
> > > #define ESDHC_TUNE_CTRL_MAX ((1 << 7) - 1)
> > >
> > > +/* strobe dll register */
> > > +#define ESDHC_STROBE_DLL_CTRL0x70
> > > +#define ESDHC_STROBE_DLL_CTRL_ENABLE (1 << 0)
> > > +#define ESDHC_STROBE_DLL_CTRL_RESET (1 << 1)
> > > +#define ESDHC_STROBE_DLL_CTRL_SLV_DLY_TARGET_SHIFT 3
> > > +
> > > +#define ESDHC_STROBE_DLL_STATUS 0x74
> > > +#define ESDHC_STROBE_DLL_STS_REF_LOCK(1 << 1)
> > > +#define ESDHC_STROBE_DLL_STS_SLV_LOCK0x1
> > > +
> > > #define ESDHC_TUNING_CTRL0xcc
> > > #define ESDHC_STD_TUNING_EN (1 << 24)
> > > /* NOTE: the minimum valid tuning start tap for mx6sl is 1 */ @@
> > > -120,6 +131,11 @@
> > > #define ESDHC_FLAG_ERR004536 BIT(7)
> > > /* The IP supports HS200 mode */
> > > #define ESDHC_FLAG_HS200 BIT(8)
> > > +/* The IP supports HS400 mode */
> > > +#define ESDHC_FLAG_HS400 BIT(9)
> > > +
> > > +/* A higher clock ferquency than this rate requires strobell dll
> > control */
> > > +#define ESDHC_STROBE_DLL_CLK_FREQ1
> > >
> > > struct esdhc_soc_data {
> > > u32 flags;
> > > @@ -156,6 +172,12 @@ static struct esdhc_soc_data usdhc_imx6sx_data = {
> > > | ESDHC_FLAG_HAVE_CAP1 | ESDHC_FLAG_HS200, };
> > >
> > > +static struct esdhc_soc_data usdhc_imx7d_data = {
> > > + .flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_STD_TUNING
> > > + | ESDHC_FLAG_HAVE_CAP1 | ESDHC_FLAG_HS200
> > > + | ESDHC_FLAG_HS400,
> > > +};
> > > +
> > > struct pltfm_imx_data {
> > > u32 scratchpad;
> > > struct pinctrl *pinctrl;
> > > @@ -199,6 +221,7 @@ static const struct of_device_id imx_esdhc_dt_ids[]
> > = {
> > > { .compatible = "fsl,imx6sx-usdhc", .data = &usdhc_imx6sx_data, },
> > > { .compatible = "fsl,imx6sl-usdhc", .data = &usdhc_imx6sl_data, },
> > > { .compatible = "fsl,imx6q-usdhc", .data = &usdhc_imx6q_data, },
> > > + { .compatible = "fsl,imx7d-usdhc", .data = &usdhc_imx7d_data, },
> > > { /* sentinel */ }
> > > };
> > > MODULE_DEVICE_TABLE(of, imx_esdhc_dt_ids); @@ -274,6 +297,9 @@ static
> > > u32 esdhc_readl_le(struct sdhci_host *host, int reg)
> > > val = SDHCI_SUPPORT_DDR50 | SDHCI_SUPPORT_SDR104
> > > | SDHCI_SUPPORT_SDR50
> > > | SDHCI_USE_SDR50_TUNING;
> > > +
> > > + if (imx_data->socdata->flags & ESDHC_FLAG_HS400)
> > > + val |= SDHCI_SUPPORT_HS400;
> > > }
> > > }
> > >
> > > @@ -774,6 +800,7 @@ static int esdhc_change_pinstate(struct sdhci_host
> > *host,
> > > break;
> > > case MMC_TIMING_UHS_SDR104:
> > > case MMC_TIMING_MMC_HS200:
> > > + case MMC_TIMING_MMC_HS400:
> > > pinctrl = imx_data->pins_200mhz;
> > > break;
> > > default:
> > > @@ -784,6 +811,44 @@ static int esdhc_change_pinstate(struct sdhci_host
> > *host,
> > > return pinctrl_select_state(imx_data->pinctrl, pinctrl); }
> > >
> > > +/*
> > > + * For HS400 eMMC, the
Re: [PATCH] mmc: sdhci: fix dma memory leak in sdhci_pre_req()
On 08/07/2015, 11:29 AM, Haibo Chen wrote:
> Currently one mrq->data maybe execute dma_map_sg() twice
> when mmc subsystem prepare over one new request, and the
> following log show up:
> sdhci[sdhci_pre_dma_transfer] invalid cookie: 24, next-cookie 25
>
> In this condition, mrq->date map a dma-memory(1) in sdhci_pre_req
> for the first time, and map another dma-memory(2) in sdhci_prepare_data
> for the second time. But driver only unmap the dma-memory(2), and
> dma-memory(1) never unmapped, which cause the dma memory leak issue.
>
> This patch use another method to map the dma memory for the mrq->data
> which can fix this dma memory leak issue.
>
> Fixes: commit 348487cb28e66b0 ("mmc: sdhci: use pipeline mmc requests to
> improve performance")
> Cc: sta...@vger.kernel.org # 4.0+
> Reported-by: Jiri Slaby
Reported-and-tested-by: Jiri Slaby
I also added the mentioned WARN_ON in place and it never triggerred. (As
I expected...)
Thanks!
--
js
suse labs
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] mmc: sdhci: fix dma memory leak in sdhci_pre_req()
On 08/07/2015, 11:29 AM, Haibo Chen wrote:
> Currently one mrq->data maybe execute dma_map_sg() twice
> when mmc subsystem prepare over one new request, and the
> following log show up:
> sdhci[sdhci_pre_dma_transfer] invalid cookie: 24, next-cookie 25
Thanks, I will test it shortly.
> @@ -2157,16 +2147,10 @@ static void sdhci_pre_req(struct mmc_host *mmc,
> struct mmc_request *mrq,
> {
> struct sdhci_host *host = mmc_priv(mmc);
>
> - if (mrq->data->host_cookie) {
> - mrq->data->host_cookie = 0;
> - return;
> - }
> + mrq->data->host_cookie = COOKIE_UNMAPPED;
Just one question. Should we warn if host_cookie != COOKIE_UNMAPPED
instead of the assignment? In other words, if the assignment is
mandatory, it deserves a comment why.
thanks,
--
js
suse labs
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] mmc: sdhci: fix dma memory leak in sdhci_pre_req()
Currently one mrq->data maybe execute dma_map_sg() twice
when mmc subsystem prepare over one new request, and the
following log show up:
sdhci[sdhci_pre_dma_transfer] invalid cookie: 24, next-cookie 25
In this condition, mrq->date map a dma-memory(1) in sdhci_pre_req
for the first time, and map another dma-memory(2) in sdhci_prepare_data
for the second time. But driver only unmap the dma-memory(2), and
dma-memory(1) never unmapped, which cause the dma memory leak issue.
This patch use another method to map the dma memory for the mrq->data
which can fix this dma memory leak issue.
Fixes: commit 348487cb28e66b0 ("mmc: sdhci: use pipeline mmc requests to
improve performance")
Cc: sta...@vger.kernel.org # 4.0+
Reported-by: Jiri Slaby
Signed-off-by: Haibo Chen
---
drivers/mmc/host/sdhci.c | 67 ++--
drivers/mmc/host/sdhci.h | 8 +++---
2 files changed, 29 insertions(+), 46 deletions(-)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index c83d110..8d2864b 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -54,8 +54,7 @@ static void sdhci_finish_command(struct sdhci_host *);
static int sdhci_execute_tuning(struct mmc_host *mmc, u32 opcode);
static void sdhci_enable_preset_value(struct sdhci_host *host, bool enable);
static int sdhci_pre_dma_transfer(struct sdhci_host *host,
- struct mmc_data *data,
- struct sdhci_host_next *next);
+ struct mmc_data *data);
static int sdhci_do_get_cd(struct sdhci_host *host);
#ifdef CONFIG_PM
@@ -495,7 +494,7 @@ static int sdhci_adma_table_pre(struct sdhci_host *host,
goto fail;
BUG_ON(host->align_addr & host->align_mask);
- host->sg_count = sdhci_pre_dma_transfer(host, data, NULL);
+ host->sg_count = sdhci_pre_dma_transfer(host, data);
if (host->sg_count < 0)
goto unmap_align;
@@ -634,9 +633,11 @@ static void sdhci_adma_table_post(struct sdhci_host *host,
}
}
- if (!data->host_cookie)
+ if (data->host_cookie == COOKIE_MAPPED) {
dma_unmap_sg(mmc_dev(host->mmc), data->sg,
data->sg_len, direction);
+ data->host_cookie = COOKIE_UNMAPPED;
+ }
}
static u8 sdhci_calc_timeout(struct sdhci_host *host, struct mmc_command *cmd)
@@ -832,7 +833,7 @@ static void sdhci_prepare_data(struct sdhci_host *host,
struct mmc_command *cmd)
} else {
int sg_cnt;
- sg_cnt = sdhci_pre_dma_transfer(host, data, NULL);
+ sg_cnt = sdhci_pre_dma_transfer(host, data);
if (sg_cnt <= 0) {
/*
* This only happens when someone fed
@@ -948,11 +949,13 @@ static void sdhci_finish_data(struct sdhci_host *host)
if (host->flags & SDHCI_USE_ADMA)
sdhci_adma_table_post(host, data);
else {
- if (!data->host_cookie)
+ if (data->host_cookie == COOKIE_MAPPED) {
dma_unmap_sg(mmc_dev(host->mmc),
data->sg, data->sg_len,
(data->flags & MMC_DATA_READ) ?
DMA_FROM_DEVICE : DMA_TO_DEVICE);
+ data->host_cookie = COOKIE_UNMAPPED;
+ }
}
}
@@ -2105,49 +2108,36 @@ static void sdhci_post_req(struct mmc_host *mmc, struct
mmc_request *mrq,
struct mmc_data *data = mrq->data;
if (host->flags & SDHCI_REQ_USE_DMA) {
- if (data->host_cookie)
+ if (data->host_cookie == COOKIE_GIVEN ||
+ data->host_cookie == COOKIE_MAPPED)
dma_unmap_sg(mmc_dev(host->mmc), data->sg, data->sg_len,
data->flags & MMC_DATA_WRITE ?
DMA_TO_DEVICE : DMA_FROM_DEVICE);
- mrq->data->host_cookie = 0;
+ data->host_cookie = COOKIE_UNMAPPED;
}
}
static int sdhci_pre_dma_transfer(struct sdhci_host *host,
- struct mmc_data *data,
- struct sdhci_host_next *next)
+ struct mmc_data *data)
{
int sg_count;
- if (!next && data->host_cookie &&
- data->host_cookie != host->next_data.cookie) {
- pr_debug(DRIVER_NAME "[%s] invalid cookie: %d, next-cookie
%d\n",
- __func__, data->host_cookie, host->next_data.cookie);
- data->host_cookie = 0;
+ if (data->host_cookie == COOKIE_MAPPED) {
+ data->host
Re: [PATCH v4 6/6] mmc: sdhci-esdhc-imx: set back the burst_length_enable bit to 1
On Wed, Aug 05, 2015 at 06:38:42PM +0800, Haibo Chen wrote: > Currently we find that if a usdhc is choosed to boot system, then ROM > code will set the burst length enable bit of this usdhc as 0. > > This will make performance drop a lot if this usdhc's burst length is > configed. So this patch set back the burst_length_enable bit as 1, > which is the default value, and means burst length is enabled for INCR. > This patch should be put before patch 5 to avoid function break, right? [PATCH v4 5/6] mmc: sdhci-esdhc-imx: change default watermark level and burst length Other than that, this patch looks good to me. Regards Dong Aisheng > Signed-off-by: Haibo Chen > --- > drivers/mmc/host/sdhci-esdhc-imx.c | 16 > 1 file changed, 16 insertions(+) > > diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c > b/drivers/mmc/host/sdhci-esdhc-imx.c > index 97aa944..3334762 100644 > --- a/drivers/mmc/host/sdhci-esdhc-imx.c > +++ b/drivers/mmc/host/sdhci-esdhc-imx.c > @@ -32,6 +32,7 @@ > #include "sdhci-esdhc.h" > > #define ESDHC_CTRL_D3CD 0x08 > +#define ESDHC_BURST_LEN_EN_INCR (1 << 27) > /* VENDOR SPEC register */ > #define ESDHC_VENDOR_SPEC0xc0 > #define ESDHC_VENDOR_SPEC_SDIO_QUIRK(1 << 1) > @@ -1165,6 +1166,21 @@ static int sdhci_esdhc_imx_probe(struct > platform_device *pdev) > host->quirks2 |= SDHCI_QUIRK2_PRESET_VALUE_BROKEN; > host->mmc->caps |= MMC_CAP_1_8V_DDR; > > + /* > + * ROM code will change the bit burst_length_enable setting > + * to zero if this usdhc is choosed to boot system. Change > + * it back here, otherwise it will impact the performance a > + * lot. This bit is used to enable/disable the burst length > + * for the external AHB2AXI bridge, it's usefully especially > + * for INCR transfer because without burst length indicator, > + * the AHB2AXI bridge does not know the burst length in > + * advance. And without burst length indicator, AHB INCR > + * transfer can only be converted to singles on the AXI side. > + */ > + writel(readl(host->ioaddr + SDHCI_HOST_CONTROL) > + | ESDHC_BURST_LEN_EN_INCR, > + host->ioaddr + SDHCI_HOST_CONTROL); > + > if (!(imx_data->socdata->flags & ESDHC_FLAG_HS200)) > host->quirks2 |= SDHCI_QUIRK2_BROKEN_HS200; > > -- > 1.9.1 > -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 2/6] mmc: sdhci-esdhc-imx: add tuning-step seting support
On Wed, Aug 05, 2015 at 06:38:38PM +0800, Haibo Chen wrote:
> tuning-step is the delay cell steps in tuning procedure. The default value
> of tuning-step is 1. Some boards or cards need another value to pass the
> tuning procedure. For example, imx7d-sdb board need the tuning-step value
> as 2, otherwise it can't pass the tuning procedure.
>
> So this patch add the tuning-step setting in driver, so that user can set
> the tuning-step value in dts.
>
> Signed-off-by: Haibo Chen
> ---
> drivers/mmc/host/sdhci-esdhc-imx.c | 9 +
> include/linux/platform_data/mmc-esdhc-imx.h | 1 +
> 2 files changed, 10 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c
> b/drivers/mmc/host/sdhci-esdhc-imx.c
> index 48f009c..803d24f 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -75,6 +75,7 @@
> #define ESDHC_STD_TUNING_EN (1 << 24)
> /* NOTE: the minimum valid tuning start tap for mx6sl is 1 */
> #define ESDHC_TUNING_START_TAP 0x1
> +#define ESDHC_TUNING_STEP_SHIFT 16
>
> /* pinctrl state */
> #define ESDHC_PINCTRL_STATE_100MHZ "state_100mhz"
> @@ -474,6 +475,7 @@ static void esdhc_writew_le(struct sdhci_host *host, u16
> val, int reg)
> } else if (imx_data->socdata->flags & ESDHC_FLAG_STD_TUNING) {
> u32 v = readl(host->ioaddr + SDHCI_ACMD12_ERR);
> u32 m = readl(host->ioaddr + ESDHC_MIX_CTRL);
> + u32 tuning_ctrl;
> if (val & SDHCI_CTRL_TUNED_CLK) {
> v |= ESDHC_MIX_CTRL_SMPCLK_SEL;
> } else {
> @@ -484,6 +486,11 @@ static void esdhc_writew_le(struct sdhci_host *host, u16
> val, int reg)
> if (val & SDHCI_CTRL_EXEC_TUNING) {
> v |= ESDHC_MIX_CTRL_EXE_TUNE;
> m |= ESDHC_MIX_CTRL_FBCLK_SEL;
> + tuning_ctrl = readl(host->ioaddr +
> ESDHC_TUNING_CTRL);
> + tuning_ctrl |= ESDHC_STD_TUNING_EN |
> ESDHC_TUNING_START_TAP;
> + if (imx_data->boarddata.tuning_step)
> + tuning_ctrl |=
> imx_data->boarddata.tuning_step << ESDHC_TUNING_STEP_SHIFT;
> + writel(tuning_ctrl, host->ioaddr +
> ESDHC_TUNING_CTRL);
Is there a code indent issue here?
Otherwise, the patch looks good to me.
Regards
Dong Aisheng
> } else {
> v &= ~ESDHC_MIX_CTRL_EXE_TUNE;
> }
> @@ -964,6 +971,8 @@ sdhci_esdhc_imx_probe_dt(struct platform_device *pdev,
> if (gpio_is_valid(boarddata->wp_gpio))
> boarddata->wp_type = ESDHC_WP_GPIO;
>
> + of_property_read_u32(np, "fsl,tuning-step", &boarddata->tuning_step);
> +
> if (of_find_property(np, "no-1-8-v", NULL))
> boarddata->support_vsel = false;
> else
> diff --git a/include/linux/platform_data/mmc-esdhc-imx.h
> b/include/linux/platform_data/mmc-esdhc-imx.h
> index e1571ef..95ccab3 100644
> --- a/include/linux/platform_data/mmc-esdhc-imx.h
> +++ b/include/linux/platform_data/mmc-esdhc-imx.h
> @@ -45,5 +45,6 @@ struct esdhc_platform_data {
> int max_bus_width;
> bool support_vsel;
> unsigned int delay_line;
> + unsigned int tuning_step; /* The delay cell steps in tuning
> procedure */
> };
> #endif /* __ASM_ARCH_IMX_ESDHC_H */
> --
> 1.9.1
>
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 1/6] mmc: sdhci-esdhc-imx: add imx7d support and support HS400
On Wed, Aug 05, 2015 at 06:38:37PM +0800, Haibo Chen wrote:
> The imx7d usdhc is derived from imx6sx, the difference is that
> imx7d support HS400.
>
> So introduce a new compatible string for imx7d and add HS400
> support for imx7d usdhc.
>
> Signed-off-by: Haibo Chen
> ---
> drivers/mmc/host/sdhci-esdhc-imx.c | 81
> ++
> 1 file changed, 81 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c
> b/drivers/mmc/host/sdhci-esdhc-imx.c
> index c6b9f64..48f009c 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -44,6 +44,7 @@
> #define ESDHC_MIX_CTRL_EXE_TUNE (1 << 22)
> #define ESDHC_MIX_CTRL_SMPCLK_SEL (1 << 23)
> #define ESDHC_MIX_CTRL_FBCLK_SEL(1 << 25)
> +#define ESDHC_MIX_CTRL_HS400_EN (1 << 26)
> /* Bits 3 and 6 are not SDHCI standard definitions */
> #define ESDHC_MIX_CTRL_SDHCI_MASK 0xb7
> /* Tuning bits */
> @@ -60,6 +61,16 @@
> #define ESDHC_TUNE_CTRL_MIN 0
> #define ESDHC_TUNE_CTRL_MAX ((1 << 7) - 1)
>
> +/* strobe dll register */
> +#define ESDHC_STROBE_DLL_CTRL0x70
> +#define ESDHC_STROBE_DLL_CTRL_ENABLE (1 << 0)
> +#define ESDHC_STROBE_DLL_CTRL_RESET (1 << 1)
> +#define ESDHC_STROBE_DLL_CTRL_SLV_DLY_TARGET_SHIFT 3
> +
> +#define ESDHC_STROBE_DLL_STATUS 0x74
> +#define ESDHC_STROBE_DLL_STS_REF_LOCK(1 << 1)
> +#define ESDHC_STROBE_DLL_STS_SLV_LOCK0x1
> +
> #define ESDHC_TUNING_CTRL0xcc
> #define ESDHC_STD_TUNING_EN (1 << 24)
> /* NOTE: the minimum valid tuning start tap for mx6sl is 1 */
> @@ -120,6 +131,11 @@
> #define ESDHC_FLAG_ERR004536 BIT(7)
> /* The IP supports HS200 mode */
> #define ESDHC_FLAG_HS200 BIT(8)
> +/* The IP supports HS400 mode */
> +#define ESDHC_FLAG_HS400 BIT(9)
> +
> +/* A higher clock ferquency than this rate requires strobell dll control */
> +#define ESDHC_STROBE_DLL_CLK_FREQ1
>
> struct esdhc_soc_data {
> u32 flags;
> @@ -156,6 +172,12 @@ static struct esdhc_soc_data usdhc_imx6sx_data = {
> | ESDHC_FLAG_HAVE_CAP1 | ESDHC_FLAG_HS200,
> };
>
> +static struct esdhc_soc_data usdhc_imx7d_data = {
> + .flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_STD_TUNING
> + | ESDHC_FLAG_HAVE_CAP1 | ESDHC_FLAG_HS200
> + | ESDHC_FLAG_HS400,
> +};
> +
> struct pltfm_imx_data {
> u32 scratchpad;
> struct pinctrl *pinctrl;
> @@ -199,6 +221,7 @@ static const struct of_device_id imx_esdhc_dt_ids[] = {
> { .compatible = "fsl,imx6sx-usdhc", .data = &usdhc_imx6sx_data, },
> { .compatible = "fsl,imx6sl-usdhc", .data = &usdhc_imx6sl_data, },
> { .compatible = "fsl,imx6q-usdhc", .data = &usdhc_imx6q_data, },
> + { .compatible = "fsl,imx7d-usdhc", .data = &usdhc_imx7d_data, },
> { /* sentinel */ }
> };
> MODULE_DEVICE_TABLE(of, imx_esdhc_dt_ids);
> @@ -274,6 +297,9 @@ static u32 esdhc_readl_le(struct sdhci_host *host, int
> reg)
> val = SDHCI_SUPPORT_DDR50 | SDHCI_SUPPORT_SDR104
> | SDHCI_SUPPORT_SDR50
> | SDHCI_USE_SDR50_TUNING;
> +
> + if (imx_data->socdata->flags & ESDHC_FLAG_HS400)
> + val |= SDHCI_SUPPORT_HS400;
> }
> }
>
> @@ -774,6 +800,7 @@ static int esdhc_change_pinstate(struct sdhci_host *host,
> break;
> case MMC_TIMING_UHS_SDR104:
> case MMC_TIMING_MMC_HS200:
> + case MMC_TIMING_MMC_HS400:
> pinctrl = imx_data->pins_200mhz;
> break;
> default:
> @@ -784,6 +811,44 @@ static int esdhc_change_pinstate(struct sdhci_host *host,
> return pinctrl_select_state(imx_data->pinctrl, pinctrl);
> }
>
> +/*
> + * For HS400 eMMC, there is a data_strobe line, this signal is generated
> + * by the device and used for data output and CRC status response output
> + * in HS400 mode. The frequency of this signal follows the frequency of
> + * CLK generated by host. Host receive the data which is aligned to the
> + * edge of data_strobe line. Due to the time delay between CLK line and
> + * data_strobe line, if the delay time is larger than one clock cycle,
> + * then CLK and data_strobe line will misaligned, read error shows up.
> + * So when the CLK is higher than 100MHz, each clock cycle is short enough,
> + * host should config the delay target.
> + */
> +static void esdhc_set_strobe_dll(struct sdhci_host *host)
> +{
> + u32 v;
> +
> + if (host->mmc->actual_clock > ESDHC_STROBE_DLL_CLK_FREQ) {
> + /* force a reset on strobe dll */
> + writel(ESDHC_STROBE_DLL_CTRL_RESET,
> + host->ioaddr + ESDHC_STROBE_DLL_CTRL);
> + /*
> + * enable strobe dll ctrl and adjust the delay target
> + * for the uSDHC lo
