* [PATCH 5/10] dt: bindings: Add bindings for Marvell Xenon SD Host Controller
From: Gregory CLEMENT @ 2016-10-31 11:09 UTC (permalink / raw)
To: Ulf Hansson, Adrian Hunter, linux-mmc
Cc: Jason Cooper, Andrew Lunn, Sebastian Hesselbarth, Gregory CLEMENT,
Rob Herring, devicetree, Thomas Petazzoni, linux-arm-kernel,
Ziji Hu, Jack(SH) Zhu, Jimmy Xu, Jisheng Zhang, Nadav Haklai,
Ryan Gao, Doug Jones, Shiwu Zhang, Victor Gu, Wei(SOCP) Liu,
Wilson Ding, Xueping Liu, Hilbert Zhang
In-Reply-To: <cover.86006f271b60cf7c0b4c5a51762a9dacca4c4718.1477911954.git-series.gregory.clement@free-electrons.com>
From: Ziji Hu <huziji@marvell.com>
Marvell Xenon SDHC can support eMMC/SD/SDIO.
Add Xenon-specific properties.
Also add properties for Xenon PHY setting.
Signed-off-by: Hu Ziji <huziji@marvell.com>
Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---
Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt | 161 +++++++-
MAINTAINERS | 1 +-
2 files changed, 162 insertions(+), 0 deletions(-)
create mode 100644 Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt
diff --git a/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt b/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt
new file mode 100644
index 000000000000..0d2d139494d3
--- /dev/null
+++ b/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt
@@ -0,0 +1,161 @@
+Marvell's Xenon SDHCI Controller device tree bindings
+This file documents differences between the core mmc properties
+described by mmc.txt and the properties used by the Xenon implementation.
+
+A single Xenon IP can support multiple slots.
+Each slot acts as an independent SDHC. It owns independent resources, such
+as register sets clock and PHY.
+Each slot should have an independent device tree node.
+
+Required Properties:
+- compatible: should be one of the following
+ - "marvell,armada-3700-sdhci": For controllers on Armada-3700 SOC.
+ Must provide a second register area and marvell,pad-type.
+ - "marvell,xenon-sdhci": For controllers on all the SOCs, other than
+ Armada-3700.
+
+- clocks:
+ Array of clocks required for SDHCI.
+ Requires at least one for Xenon IP core.
+ Some SOCs require additional clock for AXI bus.
+
+- clock-names:
+ Array of names corresponding to clocks property.
+ The input clock for Xenon IP core should be named as "core".
+ The optional AXI clock should be named as "axi".
+
+- reg:
+ * For "marvell,xenon-sdhci", one register area for Xenon IP.
+
+ * For "marvell,armada-3700-sdhci", two register areas.
+ The first one for Xenon IP register. The second one for the Armada 3700 SOC
+ PHY PAD Voltage Control register.
+ Please follow the examples with compatible "marvell,armada-3700-sdhci"
+ in below.
+ Please also check property marvell,pad-type in below.
+
+Optional Properties:
+- marvell,xenon-slotno:
+ Indicate the corresponding bit index of current Xenon SDHC slot in
+ SDHC System Operation Control Register Bit[7:0].
+ Set/clear the corresponding bit to enable/disable current Xenon SDHC
+ slot.
+ If this property is not provided, Xenon IP should contain only one
+ slot.
+
+- marvell,xenon-phy-type:
+ Xenon support mutilple types of PHYs.
+ To select eMMC 5.1 PHY, set:
+ marvell,xenon-phy-type = "emmc 5.1 phy"
+ eMMC 5.1 PHY is the default choice if this property is not provided.
+ To select eMMC 5.0 PHY, set:
+ marvell,xenon-phy-type = "emmc 5.0 phy"
+ To select SDH PHY, set:
+ marvell,xenon-phy-type = "sdh phy"
+ Please note that eMMC PHY is a general PHY for eMMC/SD/SDIO, other than for
+ eMMC only.
+
+- marvell,xenon-phy-znr:
+ Set PHY ZNR value.
+ Only available for eMMC PHY 5.1 and eMMC PHY 5.0.
+ valid range = [0:0x1F].
+ ZNR is set as 0xF by default if this property is not provided.
+
+- marvell,xenon-phy-zpr:
+ Set PHY ZPR value.
+ Only available for eMMC PHY 5.1 and eMMC PHY 5.0.
+ valid range = [0:0x1F].
+ ZPR is set as 0xF by default if this property is not provided.
+
+- marvell,xenon-phy-nr-success-tun:
+ Set the number of required consecutive successful sampling points used to
+ identify a valid sampling window, in tuning process.
+ Valid range = [1:7]. Set as 0x4 by default if this property is not provided.
+
+- marvell,xenon-phy-tun-step-divider:
+ Set the divider for calculating TUN_STEP.
+ Set as 64 by default if this property is not provided.
+
+- marvell,xenon-phy-slow-mode:
+ Force PHY into slow mode.
+ Only available when bus frequency lower than 50MHz in SDR mde.
+ Disabled by default. Please do not enable it unless it is necessary.
+
+- marvell,xenon-mask-conflict-err:
+ Mask Conflict Error alert on some SOC. Disabled by default.
+
+- marvell,xenon-tun-count:
+ Xenon SDHC SOC usually doesn't provide re-tuning counter in
+ Capabilities Register 3 Bit[11:8].
+ This property provides the re-tuning counter.
+ If this property is not set, default re-tuning counter will
+ be set as 0x9 in driver.
+
+- marvell,pad-type:
+ Type of Armada 3700 SOC PHY PAD Voltiage Controller register.
+ Only valid when "marvell,armada-3700-sdhci" is selected.
+ Two types: "sd" and "fixed-1-8v".
+ If "sd" is slected, SOC PHY PAD is set as 3.3V at the beginning and is
+ switched to 1.8V when SD in UHS-I.
+ If "fixed-1-8v" is slected, SOC PHY PAD is fixed 1.8V, such as for eMMC.
+ Please follow the examples with compatible "marvell,armada-3700-sdhci"
+ in below.
+
+Example:
+- For eMMC slot:
+
+ sdhci@aa0000 {
+ compatible = "marvell,xenon-sdhci";
+ reg = <0xaa0000 0x1000>;
+ interrupts = <GIC_SPI 13 IRQ_TYPE_LEVEL_HIGH>
+ clocks = <&emmc_clk>, <&axi_clock>;
+ clock-names = "core", "axi";
+ bus-width = <8>;
+ marvell,xenon-emmc;
+ marvell,xenon-slotno = <0>;
+ marvell,xenon-phy-type = "emmc 5.1 phy";
+ marvell,xenon-tun-count = <11>;
+ };
+
+- For SD/SDIO slot:
+
+ sdhci@ab0000 {
+ compatible = "marvell,xenon-sdhci";
+ reg = <0xab0000 0x1000>;
+ interrupts = <GIC_SPI 55 IRQ_TYPE_LEVEL_HIGH>
+ vqmmc-supply = <&sd_regulator>;
+ clocks = <&sdclk>;
+ clock-names = "core";
+ bus-width = <4>;
+ marvell,xenon-tun-count = <9>;
+ };
+
+- For eMMC slot with compatible "marvell,armada-3700-sdhci":
+
+ sdhci@aa0000 {
+ compatible = "marvell,armada-3700-sdhci";
+ reg = <0xaa0000 0x1000>,
+ <phy_addr 0x4>;
+ interrupts = <GIC_SPI 13 IRQ_TYPE_LEVEL_HIGH>
+ clocks = <&emmcclk>;
+ clock-names = "core";
+ bus-width = <8>;
+ marvell,xenon-emmc;
+
+ marvell,pad-type = "fixed-1-8v";
+ };
+
+- For SD/SDIO slot with compatible "marvell,armada-3700-sdhci":
+
+ sdhci@ab0000 {
+ compatible = "marvell,armada-3700-sdhci";
+ reg = <0xab0000 0x1000>,
+ <phy_addr 0x4>;
+ interrupts = <GIC_SPI 55 IRQ_TYPE_LEVEL_HIGH>
+ vqmmc-supply = <&sd_regulator>;
+ clocks = <&sdclk>;
+ clock-names = "core";
+ bus-width = <4>;
+
+ marvell,pad-type = "sd";
+ };
diff --git a/MAINTAINERS b/MAINTAINERS
index 1a5c4c30ea24..850a0afb0c8d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7608,6 +7608,7 @@ MARVELL XENON MMC/SD/SDIO HOST CONTROLLER DRIVER
M: Ziji Hu <huziji@marvell.com>
L: linux-mmc@vger.kernel.org
S: Supported
+F: Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt
MATROX FRAMEBUFFER DRIVER
L: linux-fbdev@vger.kernel.org
--
git-series 0.8.10
^ permalink raw reply related
* [PATCH 4/10] MAINTAINERS: add entry for Marvell Xenon MMC Host Controller drivers
From: Gregory CLEMENT @ 2016-10-31 11:09 UTC (permalink / raw)
To: Ulf Hansson, Adrian Hunter, linux-mmc
Cc: Jason Cooper, Andrew Lunn, Sebastian Hesselbarth, Gregory CLEMENT,
Rob Herring, devicetree, Thomas Petazzoni, linux-arm-kernel,
Ziji Hu, Jack(SH) Zhu, Jimmy Xu, Jisheng Zhang, Nadav Haklai,
Ryan Gao, Doug Jones, Shiwu Zhang, Victor Gu, Wei(SOCP) Liu,
Wilson Ding, Xueping Liu, Hilbert Zhang
In-Reply-To: <cover.86006f271b60cf7c0b4c5a51762a9dacca4c4718.1477911954.git-series.gregory.clement@free-electrons.com>
From: Ziji Hu <huziji@marvell.com>
Add maintainer entry for Marvell Xenon eMMC/SD/SDIO Host
Controller drivers.
Signed-off-by: Hu Ziji <huziji@marvell.com>
Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---
MAINTAINERS | 5 +++++
1 file changed, 5 insertions(+), 0 deletions(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index c44795306342..1a5c4c30ea24 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7604,6 +7604,11 @@ M: Nicolas Pitre <nico@fluxnic.net>
S: Odd Fixes
F: drivers/mmc/host/mvsdio.*
+MARVELL XENON MMC/SD/SDIO HOST CONTROLLER DRIVER
+M: Ziji Hu <huziji@marvell.com>
+L: linux-mmc@vger.kernel.org
+S: Supported
+
MATROX FRAMEBUFFER DRIVER
L: linux-fbdev@vger.kernel.org
S: Orphan
--
git-series 0.8.10
^ permalink raw reply related
* [PATCH 3/10] mmc: sdhci: Export sdhci_execute_tuning() in sdhci.c
From: Gregory CLEMENT @ 2016-10-31 11:09 UTC (permalink / raw)
To: Ulf Hansson, Adrian Hunter, linux-mmc
Cc: Jason Cooper, Andrew Lunn, Sebastian Hesselbarth, Gregory CLEMENT,
Rob Herring, devicetree, Thomas Petazzoni, linux-arm-kernel,
Ziji Hu, Jack(SH) Zhu, Jimmy Xu, Jisheng Zhang, Nadav Haklai,
Ryan Gao, Doug Jones, Shiwu Zhang, Victor Gu, Wei(SOCP) Liu,
Wilson Ding, Xueping Liu, Hilbert Zhang
In-Reply-To: <cover.86006f271b60cf7c0b4c5a51762a9dacca4c4718.1477911954.git-series.gregory.clement@free-electrons.com>
From: Ziji Hu <huziji@marvell.com>
Export sdhci_execute_tuning() from sdhci.c.
Thus vendor sdhci driver can execute its own tuning process.
Signed-off-by: Hu Ziji <huziji@marvell.com>
Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---
drivers/mmc/host/sdhci.c | 3 ++-
drivers/mmc/host/sdhci.h | 1 +
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 8e6e4e37e3b4..e971abb1368f 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1950,7 +1950,7 @@ static int sdhci_prepare_hs400_tuning(struct mmc_host *mmc, struct mmc_ios *ios)
return 0;
}
-static int sdhci_execute_tuning(struct mmc_host *mmc, u32 opcode)
+int sdhci_execute_tuning(struct mmc_host *mmc, u32 opcode)
{
struct sdhci_host *host = mmc_priv(mmc);
u16 ctrl;
@@ -2139,6 +2139,7 @@ static int sdhci_execute_tuning(struct mmc_host *mmc, u32 opcode)
spin_unlock_irqrestore(&host->lock, flags);
return err;
}
+EXPORT_SYMBOL_GPL(sdhci_execute_tuning);
static int sdhci_select_drive_strength(struct mmc_card *card,
unsigned int max_dtr, int host_drv,
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index cd18b6f19c3b..95beadc66849 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -691,6 +691,7 @@ void sdhci_set_uhs_signaling(struct sdhci_host *host, unsigned timing);
void sdhci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios);
int sdhci_start_signal_voltage_switch(struct mmc_host *mmc,
struct mmc_ios *ios);
+int sdhci_execute_tuning(struct mmc_host *mmc, u32 opcode);
#ifdef CONFIG_PM
extern int sdhci_suspend_host(struct sdhci_host *host);
--
git-series 0.8.10
^ permalink raw reply related
* [PATCH 1/10] mmc: sdhci: Export sdhci_set_ios() from sdhci.c
From: Gregory CLEMENT @ 2016-10-31 11:09 UTC (permalink / raw)
To: Ulf Hansson, Adrian Hunter, linux-mmc-u79uwXL29TY76Z2rM5mHXA
Cc: Jason Cooper, Andrew Lunn, Sebastian Hesselbarth, Gregory CLEMENT,
Rob Herring, devicetree-u79uwXL29TY76Z2rM5mHXA, Thomas Petazzoni,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Ziji Hu,
Jack(SH) Zhu, Jimmy Xu, Jisheng Zhang, Nadav Haklai, Ryan Gao,
Doug Jones, Shiwu Zhang, Victor Gu, Wei(SOCP) Liu, Wilson Ding,
Xueping Liu, Hilbert Zhang
In-Reply-To: <cover.86006f271b60cf7c0b4c5a51762a9dacca4c4718.1477911954.git-series.gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
From: Ziji Hu <huziji-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>
Export sdhci_set_ios() in sdhci.c.
Thus vendor sdhci driver can implement its own set_ios() routine.
Signed-off-by: Hu Ziji <huziji-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>
Signed-off-by: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
drivers/mmc/host/sdhci.c | 3 ++-
drivers/mmc/host/sdhci.h | 1 +
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 71654b90227f..ea06faf8a437 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1563,7 +1563,7 @@ void sdhci_set_uhs_signaling(struct sdhci_host *host, unsigned timing)
}
EXPORT_SYMBOL_GPL(sdhci_set_uhs_signaling);
-static void sdhci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
+void sdhci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
{
struct sdhci_host *host = mmc_priv(mmc);
unsigned long flags;
@@ -1723,6 +1723,7 @@ static void sdhci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
mmiowb();
spin_unlock_irqrestore(&host->lock, flags);
}
+EXPORT_SYMBOL_GPL(sdhci_set_ios);
static int sdhci_get_cd(struct mmc_host *mmc)
{
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 766df17fb7eb..37771de4cafa 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -688,6 +688,7 @@ void sdhci_set_power_noreg(struct sdhci_host *host, unsigned char mode,
void sdhci_set_bus_width(struct sdhci_host *host, int width);
void sdhci_reset(struct sdhci_host *host, u8 mask);
void sdhci_set_uhs_signaling(struct sdhci_host *host, unsigned timing);
+void sdhci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios);
#ifdef CONFIG_PM
extern int sdhci_suspend_host(struct sdhci_host *host);
--
git-series 0.8.10
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: [PATCH] mmc: Hynix: add QUIRK_NOTIFY_POWEROFF_ON_SLEEP
From: Alex Lemberg @ 2016-10-31 10:14 UTC (permalink / raw)
To: Ulf Hansson, Thierry Escande; +Cc: linux-mmc
In-Reply-To: <CAPDyKFq+t4-m3V9Qx37x8B6_FSV3C_vJtQyaEu5+fgSffcEyDg@mail.gmail.com>
Hi,
By the eMMC5.0 spec, before sending Sleep command (CMD5), hosts may set the
POWER_OFF_NOTIFICATION byte to SLEEP_NOTIFICATION (0x04).
Isn’t this a case in this patch?
If yes, why not sending SLEEP_NOTIFICATION instead of POWER_OFF_SHORT?
In the past we had a discussion on this topic, but the solution became more complicated because
of possibly long timeout (max 83 seconds) on SLEEP_NOTIFICATION.
https://marc.info/?t=143374696600002&r=1&w=1
But still, in case we want to support POWER_OFF_NOTIFICATION before Sleep command,
and in case we want to be eMMC spec aligned, I believe the right thing to do
is to support SLEEP_NOTIFICATION…
Thanks,
Alex
On 10/27/16, 10:49 PM, "linux-mmc-owner@vger.kernel.org on behalf of Ulf Hansson" <linux-mmc-owner@vger.kernel.org on behalf of ulf.hansson@linaro.org> wrote:
>On 27 October 2016 at 17:06, Thierry Escande
><thierry.escande@collabora.com> wrote:
>> Hi Ulf,
>>
>> On 25/10/2016 12:03, Ulf Hansson wrote:
>>>
>>> On 3 October 2016 at 16:19, Thierry Escande
>>> <thierry.escande@collabora.com> wrote:
>>>>
>>>> From: zhaojohn <john.zhao@intel.com>
>>>>
>>>> Hynix eMMC devices sometimes take 50% longer to resume from sleep.
>>>> Based on a recommendation from Hynix, send a Power-Off Notification
>>>> before going to S3 to restore a resume time consistently within spec.
>>>
>>>
>>> Could you also share what mmc controller and SoC you get this results
>>> from?
>>>
>>> More precisely, are you using MMC_CAP_WAIT_WHILE_BUSY?
>>
>> This occurs on a braswell based chromebook, using the acpi sdhci controller.
>> So yes, using MMC_CAP_WAIT_WHILE_BUSY.
>>
>> [...]
>>>>
>>>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>>>> index f2d185c..46a4562 100644
>>>> --- a/drivers/mmc/core/mmc.c
>>>> +++ b/drivers/mmc/core/mmc.c
>>>> @@ -1925,8 +1925,14 @@ static int _mmc_suspend(struct mmc_host *host,
>>>> bool is_suspend)
>>>> if (mmc_can_poweroff_notify(host->card) &&
>>>> ((host->caps2 & MMC_CAP2_FULL_PWR_CYCLE) || !is_suspend))
>>>> err = mmc_poweroff_notify(host->card, notify_type);
>>>> - else if (mmc_can_sleep(host->card))
>>>> + else if (mmc_can_sleep(host->card)) {
>>>> + if (host->card->quirks &
>>>> MMC_QUIRK_NOTIFY_POWEROFF_ON_SLEEP) {
>>>> + err = mmc_poweroff_notify(host->card,
>>>> notify_type);
>>>> + if (err)
>>>> + goto out;
>>>> + }
>>>
>>>
>>> So, I am curious to know from a power management point of view; how
>>> does the card behave comparing the sleep and power off notification
>>> command?
>>>
>>> Is the card in a low power state after the power off notification has
>>> been received? If so, did you manage to do some measurement for that
>>> or perhaps the data-sheet tells about this? It would be interesting to
>>> know if there were any differences between sleep and power off
>>> notification in this regards.
>>
>> I do not have any clue about that. It appears only with Hynix emmc and the
>> fix has been approved by Hynix engineers... It seems that if not powered
>> off, the firmware does some garbage collection when resuming and it takes
>> more time...
>
>Okay, I see. So before I continue reviewing, can you please also tell
>what regulators to the card that is being cut while powering off in
>this path.
>
>VMMC, VQMMC?
>
>Kind regards
>Uffe
>--
>To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 2/3] mmc: Adding AUTO_BKOPS_EN bit set for Auto BKOPS support
From: Alex Lemberg @ 2016-10-31 9:53 UTC (permalink / raw)
To: Ulf Hansson; +Cc: linux-mmc, Avi Shchislowski
In-Reply-To: <CAPDyKFo2BzcscQtL6rgTyj8OHqv5CdB4SdyA1ob=1n14vNhHkQ@mail.gmail.com>
Hi Ulf,
On 10/13/16, 10:36 AM, "Ulf Hansson" <ulf.hansson@linaro.org> wrote:
>On 1 September 2016 at 16:24, alex lemberg <alex.lemberg@sandisk.com> wrote:
>> Signed-off-by: alex lemberg <alex.lemberg@sandisk.com>
>> ---
>> drivers/mmc/core/mmc.c | 6 ++++++
>> include/linux/mmc/card.h | 1 +
>> include/linux/mmc/mmc.h | 1 +
>> 3 files changed, 8 insertions(+)
>>
>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>> index f2d185c..e2e987f 100644
>> --- a/drivers/mmc/core/mmc.c
>> +++ b/drivers/mmc/core/mmc.c
>> @@ -531,6 +531,12 @@ static int mmc_decode_ext_csd(struct mmc_card *card, u8 *ext_csd)
>> if (!card->ext_csd.man_bkops_en)
>> pr_debug("%s: MAN_BKOPS_EN bit is not set\n",
>> mmc_hostname(card->host));
>> + card->ext_csd.auto_bkops_en =
>> + (ext_csd[EXT_CSD_BKOPS_EN] &
>> + EXT_CSD_AUTO_BKOPS_MASK);
>> + if (!card->ext_csd.auto_bkops_en)
>> + pr_debug("%s: AUTO_BKOPS_EN bit is not set\n",
>> + mmc_hostname(card->host));
>
>According to discussions here [1], I am wondering whether we actually
>should inverse the logic for when printing the debug message.
>
>Isn't it more relevant to know when the bits *are* set than rather
>when they aren't?
Agree, since we have two types of BKOPS now, it more makes sense
to print a currently enabled/set type or BKOPS bits.
I will change and re-submit.
>
>> }
>>
>> /* check whether the eMMC card supports HPI */
>> diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
>> index 73fad83..aaedb68 100644
>> --- a/include/linux/mmc/card.h
>> +++ b/include/linux/mmc/card.h
>> @@ -84,6 +84,7 @@ struct mmc_ext_csd {
>> unsigned int hpi_cmd; /* cmd used as HPI */
>> bool bkops; /* background support bit */
>> bool man_bkops_en; /* manual bkops enable bit */
>> + bool auto_bkops_en; /* auto bkops enable bit */
>> unsigned int data_sector_size; /* 512 bytes or 4KB */
>> unsigned int data_tag_unit_size; /* DATA TAG UNIT size */
>> unsigned int boot_ro_lock; /* ro lock support */
>> diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h
>> index c376209..0fe3908 100644
>> --- a/include/linux/mmc/mmc.h
>> +++ b/include/linux/mmc/mmc.h
>> @@ -436,6 +436,7 @@ struct _mmc_csd {
>> * BKOPS modes
>> */
>> #define EXT_CSD_MANUAL_BKOPS_MASK 0x01
>> +#define EXT_CSD_AUTO_BKOPS_MASK 0x02
>>
>> /*
>> * MMC_SWITCH access modes
>> --
>> 1.9.1
>>
>
>Kind regards
>Uffe
>
>[1]
>http://www.spinics.net/lists/linux-mmc/msg39553.html
^ permalink raw reply
* Re: [PATCH 3/3] mmc: Checking BKOPS status prior to Suspend
From: Alex Lemberg @ 2016-10-31 9:53 UTC (permalink / raw)
To: Ulf Hansson; +Cc: linux-mmc, Avi Shchislowski
In-Reply-To: <CAPDyKFpmbwNvx+00h=Aq-UAb9OLDtxrwr7N408L2pokxX+zgug@mail.gmail.com>
Hi Ulf,
Thank you for reviewing the patch!
On 10/13/16, 10:36 AM, "Ulf Hansson" <ulf.hansson@linaro.org> wrote:
>On 1 September 2016 at 16:24, alex lemberg <alex.lemberg@sandisk.com> wrote:
>> Rescheduling Suspend in case of BKOPS Level >= 1
>> in order to let eMMC device to complete its internal GC.
>> Applicable for Runtime Suspend Only.
>>
>> Signed-off-by: alex lemberg <alex.lemberg@sandisk.com>
>> ---
>> drivers/mmc/core/mmc.c | 30 +++++++++++++++++++++++-------
>> include/linux/mmc/mmc.h | 1 +
>> 2 files changed, 24 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>> index e2e987f..c4c6326 100644
>> --- a/drivers/mmc/core/mmc.c
>> +++ b/drivers/mmc/core/mmc.c
>> @@ -1904,7 +1904,8 @@ static void mmc_detect(struct mmc_host *host)
>> }
>> }
>>
>> -static int _mmc_suspend(struct mmc_host *host, bool is_suspend)
>> +static int _mmc_suspend(struct mmc_host *host, bool is_suspend,
>> + bool is_runtime_pm)
>> {
>> int err = 0;
>> unsigned int notify_type = is_suspend ? EXT_CSD_POWER_OFF_SHORT :
>> @@ -1918,10 +1919,25 @@ static int _mmc_suspend(struct mmc_host *host, bool is_suspend)
>> if (mmc_card_suspended(host->card))
>> goto out;
>>
>> - if (mmc_card_doing_bkops(host->card)) {
>> - err = mmc_stop_bkops(host->card);
>> - if (err)
>> + if (mmc_card_doing_bkops(host->card) ||
>> + host->card->ext_csd.auto_bkops_en) {
>> + err = mmc_read_bkops_status(host->card);
>> + if (err) {
>> + pr_err("%s: error %d reading BKOPS Status\n",
>> + mmc_hostname(host), err);
>> + goto out;
>> + }
>
>I would appreciate some simple comment of what you want to do here below.
>
>> + if (is_runtime_pm && host->card->ext_csd.raw_bkops_status >=
>> + EXT_CSD_BKOPS_LEVEL_1) {
>> + pm_schedule_suspend(&host->card->dev,
>> + MMC_RUNTIME_SUSPEND_DELAY_MS);
>> goto out;
>
>If I understand correctly, you would like to abort the runtime suspend
>and allow the background operations to complete. That seems
>reasonable, although in such case you need to return -EBUSY from this
>function.
In my mind I wanted to reschedule runtime suspend, but your recommendation
is makes sense. I will change it and re-submit.
>Moreover, perhaps we should discuss at what BKOPS_LEVEL* we should
>allow to abort.
Agree, the BKOPS_LEVEL value is something that can be interpreted differently
by different vendors.
Should I define a default value like “BKOPS_LEVEL_TO_USE”, and
also maybe add a sysfs entry to allow user configuration?
>
>> + }
>> + if (mmc_card_doing_bkops(host->card)) {
>> + err = mmc_stop_bkops(host->card);
>> + if (err)
>> + goto out;
>> + }
>> }
>>
>> err = mmc_flush_cache(host->card);
>> @@ -1952,7 +1968,7 @@ static int mmc_suspend(struct mmc_host *host)
>> {
>> int err;
>>
>> - err = _mmc_suspend(host, true);
>> + err = _mmc_suspend(host, true, false);
>> if (!err) {
>> pm_runtime_disable(&host->card->dev);
>> pm_runtime_set_suspended(&host->card->dev);
>> @@ -2002,7 +2018,7 @@ static int mmc_shutdown(struct mmc_host *host)
>> err = _mmc_resume(host);
>>
>> if (!err)
>> - err = _mmc_suspend(host, false);
>> + err = _mmc_suspend(host, false, false);
>>
>> return err;
>> }
>> @@ -2026,7 +2042,7 @@ static int mmc_runtime_suspend(struct mmc_host *host)
>> if (!(host->caps & MMC_CAP_AGGRESSIVE_PM))
>> return 0;
>>
>> - err = _mmc_suspend(host, true);
>> + err = _mmc_suspend(host, true, true);
>> if (err)
>> pr_err("%s: error %d doing aggressive suspend\n",
>> mmc_hostname(host), err);
>> diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h
>> index 0fe3908..0f08d5c 100644
>> --- a/include/linux/mmc/mmc.h
>> +++ b/include/linux/mmc/mmc.h
>> @@ -430,6 +430,7 @@ struct _mmc_csd {
>> /*
>> * BKOPS status level
>> */
>> +#define EXT_CSD_BKOPS_LEVEL_1 0x1
>> #define EXT_CSD_BKOPS_LEVEL_2 0x2
>>
>> /*
>> --
>> 1.9.1
>>
>
>Another idea I had, was to actually make use of that we know the card
>will be inactive. So instead of checking if there is an ongoing BKOPS
>(mmc_card_doing_bkops()), in case of the manual BKOPS, we could
>potentially check the needed BKOPS LEVEL and schedule a BKOPS to
>start. In other words, do manual BKOPS when the card is idle.
If I understand correctly, the idea is to allow MANUAL_ BKOPS in
a similar way as it done for AUTO_BKOPS during runtime suspend, right?
>
>Of course, one need to be able to interrupt/cancel the BKOPS if there
>is a new request that needs to be managed. I am not sure how we can
>achieve that, but I hope you get the idea.
Right, in case of MANUAL_BKOPS, the need the HPI to be issued
in order to stop MANUAL_BKOPS immediately on a new request.
BTW, the same challenge we had on SLEEP_NOTIFICATION…
Since the AUTO_BKOPS is more simple case and doesn’t require
the interruption ability from the driver, may I suggest to make it
in a separate patchset?
>
>Kind regards
>Uffe
^ permalink raw reply
* Re: [PATCH v2 2/5] mmc: zx: Initial support for ZX mmc controller
From: Jaehoon Chung @ 2016-10-31 9:40 UTC (permalink / raw)
To: Jun Nie; +Cc: Shawn Guo, xie.baoyou, Ulf Hansson, Jason Liu, linux-mmc
In-Reply-To: <CABymUCPe3dsn5U-5igUjRUSUw006XkaWLbwYp3a2mk1jSmP9=A@mail.gmail.com>
On 10/31/2016 05:47 PM, Jun Nie wrote:
> 2016-10-28 13:16 GMT+08:00 Jaehoon Chung <jh80.chung@samsung.com>:
>> On 10/28/2016 11:37 AM, Jun Nie wrote:
>>> This platform driver adds initial support for the DW host controller
>>> found on ZTE SoCs.
>>>
>>> It has been tested on ZX296718 EVB board currently. More support on
>>> timing tuning will be added when hardware is available.
>>>
>>> Signed-off-by: Jun Nie <jun.nie@linaro.org>
>>> ---
>>> drivers/mmc/host/Kconfig | 9 ++
>>> drivers/mmc/host/Makefile | 1 +
>>> drivers/mmc/host/dw_mmc-zx.c | 230 +++++++++++++++++++++++++++++++++++++++++++
>>> drivers/mmc/host/dw_mmc-zx.h | 23 +++++
>>> 4 files changed, 263 insertions(+)
>>> create mode 100644 drivers/mmc/host/dw_mmc-zx.c
>>> create mode 100644 drivers/mmc/host/dw_mmc-zx.h
>>>
>>> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
>>> index 5274f50..2b3202c 100644
>>> --- a/drivers/mmc/host/Kconfig
>>> +++ b/drivers/mmc/host/Kconfig
>>> @@ -662,6 +662,15 @@ config MMC_DW_ROCKCHIP
>>> Synopsys DesignWare Memory Card Interface driver. Select this option
>>> for platforms based on RK3066, RK3188 and RK3288 SoC's.
>>>
>>> +config MMC_DW_ZX
>>> + tristate "ZTE specific extensions for Synopsys DW Memory Card Interface"
>>> + depends on MMC_DW
>>
>> I guess MMC_DW_ZX depends on your SoC config, doesn't?
>
> Right, will add dependency.
>
>>
>>> + select MMC_DW_PLTFM
>>> + help
>>> + This selects support for ZTE SoC specific extensions to the
>>> + Synopsys DesignWare Memory Card Interface driver. Select this option
>>> + for platforms based on ZX296718 SoC's.
>>> +
>>> config MMC_SH_MMCIF
>>> tristate "SuperH Internal MMCIF support"
>>> depends on HAS_DMA
>>> diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
>>> index e2bdaaf..9766143 100644
>>> --- a/drivers/mmc/host/Makefile
>>> +++ b/drivers/mmc/host/Makefile
>>> @@ -48,6 +48,7 @@ obj-$(CONFIG_MMC_DW_EXYNOS) += dw_mmc-exynos.o
>>> obj-$(CONFIG_MMC_DW_K3) += dw_mmc-k3.o
>>> obj-$(CONFIG_MMC_DW_PCI) += dw_mmc-pci.o
>>> obj-$(CONFIG_MMC_DW_ROCKCHIP) += dw_mmc-rockchip.o
>>> +obj-$(CONFIG_MMC_DW_ZX) += dw_mmc-zx.o
>>> obj-$(CONFIG_MMC_SH_MMCIF) += sh_mmcif.o
>>> obj-$(CONFIG_MMC_JZ4740) += jz4740_mmc.o
>>> obj-$(CONFIG_MMC_VUB300) += vub300.o
>>> diff --git a/drivers/mmc/host/dw_mmc-zx.c b/drivers/mmc/host/dw_mmc-zx.c
>>> new file mode 100644
>>> index 0000000..0404f8e
>>> --- /dev/null
>>> +++ b/drivers/mmc/host/dw_mmc-zx.c
>>> @@ -0,0 +1,230 @@
>>> +/*
>>> + * ZX Specific Extensions for Synopsys DW Multimedia Card Interface driver
>>> + *
>>> + * Copyright (C) 2016, Linaro Ltd.
>>> + * Copyright (C) 2016, ZTE Corp.
>>> + *
>>> + * This program is free software; you can redistribute it and/or modify
>>> + * it under the terms of the GNU General Public License as published by
>>> + * the Free Software Foundation; either version 2 of the License, or
>>> + * (at your option) any later version.
>>> + */
>>> +
>>> +#include <linux/clk.h>
>>> +#include <linux/mfd/syscon.h>
>>> +#include <linux/mmc/dw_mmc.h>
>>> +#include <linux/mmc/host.h>
>>> +#include <linux/mmc/mmc.h>
>>> +#include <linux/module.h>
>>> +#include <linux/of.h>
>>> +#include <linux/platform_device.h>
>>> +#include <linux/pm_runtime.h>
>>> +#include <linux/regmap.h>
>>> +#include <linux/slab.h>
>>> +
>>> +#include "dw_mmc.h"
>>> +#include "dw_mmc-pltfm.h"
>>> +#include "dw_mmc-zx.h"
>>> +
>>> +#define ZX_DLL_LOCKED BIT(2)
>>
>> Some DLL rigsters and bits are defined in dw_mmc-zx.h.
>> why defined ZX_DLL_LOCKED at here.
>>
>> You can choose that all defines locates to dw_mmc-zx.c or dw_mmc-zx.h
>>
> Will move together.
>
>>> +
>>> +struct dw_mci_zx_priv_data {
>>> + struct regmap *sysc_base;
>>> +};
>>> +
>>> +static int dw_mci_zx_emmc_set_delay(struct dw_mci *host, unsigned int delay,
>>> + unsigned int clk_flag)
>>
>> Why do you use "unsigned int" as clk_flag? It's just use one of 0 and 1.
>> And 0 and 1 are what means? On/Off?
>>
> Yes, enumeration shall looks better here. It is a flag for different
> delay type. Will change to enumeration.
>
>>> +{
>>> + struct dw_mci_zx_priv_data *priv = host->priv;
>>> + struct regmap *sysc_base = priv->sysc_base;
>>> + unsigned int clksel;
>>> + unsigned int loop = 1000;
>>> + int ret;
>>> +
>>
>> priv->sysc_base doesn't never NULL?
>
> For this SoC, it is never NULL if dts is correct. Adding a check is
> better anyway.
>>
>>> + ret = regmap_write(sysc_base, LB_AON_EMMC_CFG_REG0,
>>> + PARA_DLL_START_POINT(4) | PARA_DLL_LOCK_NUM(4));
>>
>> Could you add the comment for controlling this regs?
>> I'm not sure because i didn't have ZX TRM..but PARA_DLL_LOCK_NUM should be locked?
>>
>> It doesn't affect to other bit?
>> I think you can use the regmap_update_bits instead of regmap_write.
>
> It does not affect other bit as all bits are write as desired. But
> your suggestion make it clearer.
>
>>
>>> + if (ret)
>>> + return ret;
>>> +
>>> + ret = regmap_read(sysc_base, LB_AON_EMMC_CFG_REG1, &clksel);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + if (clk_flag) {
>>> + clksel &= ~(CLK_SAMP_DELAY(0x7F));
>>
>> It's meaningless..CLK_SAMP_DELAY used only at here.
>> CLK_SAMP_DELAY ((x) & 0x7F << 0)
>>
>> It's just CLK_SAMP_DELAY_MASK.?
>>
>> #define CLK_SAMP_DELAY_MASK (0x7F << 0)
>> clksel &= ~CLK_SAMP_DELAY_MASK;
>>
> Right, clearer.
>
>>
>>> + clksel |= (delay << 8);
>>
>> Use the CLK_SAMP_DELAY_SHIFT instead of 8.
>>
>>> + } else {
>>> + clksel &= ~(READ_DQS_DELAY(0x7F));
>>
>> Ditto.
>> And i think it also can be changed to regmap_update_bits.
>>
>>> + clksel |= delay;
>>> + }
>>> +
>>> + regmap_write(sysc_base, LB_AON_EMMC_CFG_REG1, clksel);
>>> + regmap_write(sysc_base, LB_AON_EMMC_CFG_REG0,
>>> + PARA_DLL_START_POINT(4) | PARA_DLL_LOCK_NUM(4) |
>>> + DLL_REG_SET);
>>
>> regmap_update_bits?
>
> Will do.
>
>>
>>> +
>>> + do {
>>> + ret = regmap_read(sysc_base, LB_AON_EMMC_CFG_REG2, &clksel);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + } while (--loop && !(clksel & ZX_DLL_LOCKED));
>>> +
>>> + if (!loop) {
>>> + dev_err(host->dev, "Error: %s dll lock fail\n", __func__);
>>> + return -EIO;
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int dw_mci_zx_emmc_execute_tuning(struct dw_mci_slot *slot, u32 opcode)
>>> +{
>>> + struct dw_mci *host = slot->host;
>>> + struct mmc_host *mmc = slot->mmc;
>>> + int len, start = 0, end = 0, delay, best = 0;
>>> + int ret = 0;
>>> +
>>> + for (delay = 1 ; delay < 128; delay++) {
>>> + ret = dw_mci_zx_emmc_set_delay(host, delay, 1);
>>> + if (ret)
>>> + return ret;
>>
>> When it's failed, just returned.
>> Doesn't need to try with next delay value?
>
> Yes, more retry make robust.
>
>>
>>> +
>>> + if (mmc_send_tuning(mmc, opcode, NULL)) {
>>> + if (start >= 0) {
>>> + end = delay - 1;
>>> + /* check and update longest good range */
>>> + if ((end - start) > len) {
>>> + best = (start + end) >> 1;
>>> + len = end - start;
>>> + }
>>> + }
>>> + start = -1;
>>> + end = 0;
>>> + continue;
>>> + }
>>> + if (start < 0)
>>> + start = delay;
>>> + }
>>> +
>>> + if (start >= 0) {
>>> + end = delay - 1;
>>> + if ((end - start) > len) {
>>> + best = (start + end) >> 1;
>>> + len = end - start;
>>> + }
>>> + }
>>> + if (best < 0)
>>> + return -EIO;
>>> +
>>> + dev_info(host->dev, "%s best range: start %d end %d\n", __func__,
>>> + start, end);
>>> + dw_mci_zx_emmc_set_delay(host, best, 1);
>>> + return 0;
>>> +}
>>> +
>>> +static int dw_mci_zx_prepare_hs400_tuning(struct dw_mci *host,
>>> + struct mmc_ios *ios)
>>> +{
>>> + int ret;
>>> +
>>> + /* config phase shift 90 */
>>> + ret = dw_mci_zx_emmc_set_delay(host, 32, 0);
>>
>> It's always fixed to 32? What means 32?
>
> It means 90 degree shift for value 32. This configuration comes from
> ZTE engineer as I do not have hardware for this tuning. Let's just
> keep it with adding more comments till tuning is needed.
>
>>
>>> + if (ret < 0)
>>> + return -EIO;
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int dw_mci_zx_execute_tuning(struct dw_mci_slot *slot, u32 opcode)
>>> +{
>>> + struct dw_mci *host = slot->host;
>>> +
>>> + if (host->verid == 0x290a) /* emmc */
>>> + return dw_mci_zx_emmc_execute_tuning(slot, opcode);
>>
>> I didn't know why you check host->verid is 2.90a..
>> Is there any reason?
> There are two version DW MMC IP on this SoC and different
> configuration is needed for them. I do not have hardware for tuning
> version 210A timing and will be added later.
you means there are two IP version with same SoC?
Best Regards,
Jaehoon Chung
>
>>
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int dw_mci_zx_parse_dt(struct dw_mci *host)
>>> +{
>>> + struct device_node *np = host->dev->of_node;
>>> + struct device_node *node;
>>> + struct dw_mci_zx_priv_data *priv;
>>> + struct regmap *sysc_base;
>>> + int ret;
>>> +
>>> + node = of_parse_phandle(np, "zte,aon-syscon", 0);
>>> + if (node) {
>>> + sysc_base = syscon_node_to_regmap(node);
>>> + of_node_put(node);
>>
>> Use the syscon_regmap_lookup_by_phandle(). It's same behavior.
> Will do.
>>
>>> +
>>> + if (IS_ERR(sysc_base)) {
>>> + ret = PTR_ERR(sysc_base);
>>> + if (ret != -EPROBE_DEFER)
>>> + dev_err(host->dev, "Can't get syscon: %d\n",
>>> + ret);
>>> + return ret;
>>> + }
>>> + }
>>> +
>>> + priv = devm_kzalloc(host->dev, sizeof(*priv), GFP_KERNEL);
>>> + if (!priv)
>>> + return -ENOMEM;
>>> + priv->sysc_base = sysc_base;
>>
>> Is there no case that sysc_base is NULL?
>
> sysc_base is needed only for eMMC. So it is NULL for SD/MMC cases, and
> we can save memory for SD/MMC cases here :)
>
>>
>>> + host->priv = priv;
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static unsigned long zx_dwmmc_caps[3] = {
>>> + MMC_CAP_CMD23,
>>> + MMC_CAP_CMD23,
>>> + MMC_CAP_CMD23,
>>> +};
>>> +
>>> +static const struct dw_mci_drv_data zx_drv_data = {
>>> + .caps = zx_dwmmc_caps,
>>> + .execute_tuning = dw_mci_zx_execute_tuning,
>>> + .prepare_hs400_tuning = dw_mci_zx_prepare_hs400_tuning,
>>> + .parse_dt = dw_mci_zx_parse_dt,
>>> +};
>>> +
>>> +static const struct of_device_id dw_mci_zx_match[] = {
>>> + { .compatible = "zte,dw-mshc", .data = &zx_drv_data},
>>> +};
>>> +MODULE_DEVICE_TABLE(of, dw_mci_zx_match);
>>> +
>>> +static int dw_mci_zx_probe(struct platform_device *pdev)
>>> +{
>>> + const struct dw_mci_drv_data *drv_data;
>>> + const struct of_device_id *match;
>>> +
>>> + match = of_match_node(dw_mci_zx_match, pdev->dev.of_node);
>>> + drv_data = match->data;
>>> +
>>> + return dw_mci_pltfm_register(pdev, drv_data);
>>> +}
>>> +
>>> +static const struct dev_pm_ops dw_mci_zx_dev_pm_ops = {
>>> + SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
>>> + pm_runtime_force_resume)
>>> + SET_RUNTIME_PM_OPS(dw_mci_runtime_suspend,
>>> + dw_mci_runtime_resume,
>>> + NULL)
>>> +};
>>> +
>>> +static struct platform_driver dw_mci_zx_pltfm_driver = {
>>> + .probe = dw_mci_zx_probe,
>>> + .remove = dw_mci_pltfm_remove,
>>> + .driver = {
>>> + .name = "dwmmc_zx",
>>> + .of_match_table = dw_mci_zx_match,
>>> + .pm = &dw_mci_zx_dev_pm_ops,
>>> + },
>>> +};
>>> +
>>> +module_platform_driver(dw_mci_zx_pltfm_driver);
>>> +
>>> +MODULE_DESCRIPTION("ZTE emmc/sd driver");
>>> +MODULE_LICENSE("GPL v2");
>>> diff --git a/drivers/mmc/host/dw_mmc-zx.h b/drivers/mmc/host/dw_mmc-zx.h
>>> new file mode 100644
>>> index 0000000..b1aac52
>>> --- /dev/null
>>> +++ b/drivers/mmc/host/dw_mmc-zx.h
>>> @@ -0,0 +1,23 @@
>>> +#ifndef _DW_MMC_ZX_H_
>>> +#define _DW_MMC_ZX_H_
>>> +
>>> +/* dll reg offset*/
>>> +#define LB_AON_EMMC_CFG_REG0 0x1B0
>>> +#define LB_AON_EMMC_CFG_REG1 0x1B4
>>> +#define LB_AON_EMMC_CFG_REG2 0x1B8
>>> +
>>> +/* LB_AON_EMMC_CFG_REG0 register defines */
>>> +#define PARA_DLL_START_POINT(x) (((x) & 0xFF) << 0)
>>> +#define DLL_REG_SET BIT(8)
>>> +#define PARA_DLL_LOCK_NUM(x) (((x) & 7) << 16)
>>> +#define PARA_PHASE_DET_SEL(x) (((x) & 7) << 20)
>>> +#define PARA_DLL_BYPASS_MODE BIT(23)
>>> +#define PARA_HALF_CLK_MODE BIT(24)
>>
>> PAR_PHASE_DET_SEL/PARA_DLL_BYPASS_MODE_BIT/PARA_HALF_CLK_MODE are never used anywhere.
>>
>>> +
>>> +/* LB_AON_EMMC_CFG_REG1 register defines */
>>> +#define READ_DQS_DELAY(x) (((x) & 0x7F) << 0)
>>> +#define READ_DQS_BYPASS_MODE BIT(7)
>>> +#define CLK_SAMP_DELAY(x) (((x) & 0x7F) << 8)
>>> +#define CLK_SAMP_BYPASS_MODE BIT(15)
>>
>> Also READ_DQS_BYPASS_MODe/CLK_SAMP_BYBASS_MODE didnt used anywhere.
>>
>>
>> Hmm. These are not dwmmc host controller's register.
>> So If you needs to add these defines..I think you needs to add dessriptions in more detail.
>>
>> At least..Which board's DLL reg offset.
>>
>> Best Regards,
>> Jaehoon Chung
>>
>>> +
>>> +#endif /* _DW_MMC_ZX_H_ */
>>>
>>
>
>
>
^ permalink raw reply
* RE: [v15, 0/7] Fix eSDHC host version register bug
From: Y.B. Lu @ 2016-10-31 9:36 UTC (permalink / raw)
To: Arnd Bergmann,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
Cc: Mark Rutland, ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, M.H. Lian,
linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-clk-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Qiang Zhao,
Russell King, Bhupesh Sharma, Jochen Friedrich, Claudiu Manoil,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Kumar Gala,
Scott Wood, Rob Herring, Santosh Shilimkar, Greg Kroah-Hartman,
linux-mmc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, X.B. Xie,
Leo Li, iommu
In-Reply-To: <3386858.dvuMhvkN3m@wuerfel>
> -----Original Message-----
> From: Arnd Bergmann [mailto:arnd-r2nGTMty4D4@public.gmane.org]
> Sent: Friday, October 28, 2016 6:54 PM
> To: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> Cc: Y.B. Lu; linux-mmc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org; Scott
> Wood; Mark Rutland; Greg Kroah-Hartman; X.B. Xie; M.H. Lian; linux-
> i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-clk-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Qiang Zhao; Russell King;
> Bhupesh Sharma; Joerg Roedel; Jochen Friedrich; Claudiu Manoil;
> devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Rob Herring; Santosh Shilimkar;
> netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Leo Li;
> iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org; Kumar Gala; linuxppc-
> dev-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
> Subject: Re: [v15, 0/7] Fix eSDHC host version register bug
>
> On Friday, October 28, 2016 2:50:11 PM CEST Yangbo Lu wrote:
> > This patchset is used to fix a host version register bug in the
> > T4240-R1.0-R2.0 eSDHC controller. To match the SoC version and
> > revision, 10 previous version patchsets had tried many methods but all
> of them were rejected by reviewers.
> > Such as
> > - dts compatible method
> > - syscon method
> > - ifdef PPC method
> > - GUTS driver getting SVR method Anrd suggested a
> > soc_device_match method in v10, and this is the only available method
> > left now. This v11 patchset introduces the soc_device_match interface
> > in soc driver.
> >
> > The first five patches of Yangbo are to add the GUTS driver. This is
> > used to register a soc device which contain soc version and revision
> information.
> > The other two patches introduce the soc_device_match method in soc
> > driver and apply it on esdhc driver to fix this bug.
> >
>
> Looks good overall. With patch 3 dropped (or an explanation why it's
> still needed), everything
>
> Acked-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
>
[Lu Yangbo-B47093] Thank you very much:) See my explaination in patch 3 email.
> Arnd
^ permalink raw reply
* Re: [PATCH v2 4/5] mmc: dw: Add fifo address property
From: Jaehoon Chung @ 2016-10-31 9:35 UTC (permalink / raw)
To: Jun Nie; +Cc: Shawn Guo, xie.baoyou, Ulf Hansson, Jason Liu, linux-mmc
In-Reply-To: <CABymUCMAW60bvi=OGhgxoBdK8t4cXgycCp_ELUE0bEQOVL0dnA@mail.gmail.com>
On 10/31/2016 05:50 PM, Jun Nie wrote:
> 2016-10-28 13:24 GMT+08:00 Jaehoon Chung <jh80.chung@samsung.com>:
>> On 10/28/2016 11:37 AM, Jun Nie wrote:
>>> The FIFO address may break default address assumption of 0x100
>>> (version < 0x240A) and 0x200(version >= 0x240A) in current driver.
>>> The new property is introduced to override fifo address via DT
>>> node information.
>>>
>>> Signed-off-by: Jun Nie <jun.nie@linaro.org>
>>> ---
>>> drivers/mmc/host/dw_mmc.c | 5 +++++
>>> include/linux/mmc/dw_mmc.h | 2 ++
>>> 2 files changed, 7 insertions(+)
>>>
>>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>>> index 1c9ee36..24ae05b6 100644
>>> --- a/drivers/mmc/host/dw_mmc.c
>>> +++ b/drivers/mmc/host/dw_mmc.c
>>> @@ -2955,6 +2955,8 @@ static struct dw_mci_board *dw_mci_parse_dt(struct dw_mci *host)
>>>
>>> of_property_read_u32(np, "card-detect-delay", &pdata->detect_delay_ms);
>>>
>>> + of_property_read_u32(np, "fifo-addr", &host->fifo_addr_override);
>>> +
>>> if (!of_property_read_u32(np, "clock-frequency", &clock_frequency))
>>> pdata->bus_hz = clock_frequency;
>>>
>>> @@ -3163,6 +3165,9 @@ int dw_mci_probe(struct dw_mci *host)
>>> else
>>> host->fifo_reg = host->regs + DATA_240A_OFFSET;
>>>
>>> + if (host->fifo_addr_override)
>>> + host->fifo_reg = host->regs + host->fifo_addr_override;
>>> +
>>
>> Check condition the sequentially.
>>
>> if (host->fifo_addr_override) {
>> ...
>> } else if (host->verid < DW_MMC_240A) {
>> ..
>> } else {
>> ..
>> }
>>
>> how about?
>
> Okay, will change to this.
>
>>
>>> tasklet_init(&host->tasklet, dw_mci_tasklet_func, (unsigned long)host);
>>> ret = devm_request_irq(host->dev, host->irq, dw_mci_interrupt,
>>> host->irq_flags, "dw-mci", host);
>>> diff --git a/include/linux/mmc/dw_mmc.h b/include/linux/mmc/dw_mmc.h
>>> index f5af2bd..4866ef5 100644
>>> --- a/include/linux/mmc/dw_mmc.h
>>> +++ b/include/linux/mmc/dw_mmc.h
>>> @@ -107,6 +107,7 @@ struct dw_mci_dma_slave {
>>> * @ciu_clk: Pointer to card interface unit clock instance.
>>> * @slot: Slots sharing this MMC controller.
>>> * @fifo_depth: depth of FIFO.
>>> + * @fifo_addr_override: override fifo reg offset with this value.
>>
>> DATA addr is more correct. it's related with DATA register.
>
> You name it, will change to data_addr_override.
Yes, data_addr or data_addr_override? It's more clear than fifo.
Best Regards,
Jaehoon Chung
>>
>> Best Regards,
>> Jaehoon Chung
>>
>>> * @data_shift: log2 of FIFO item size.
>>> * @part_buf_start: Start index in part_buf.
>>> * @part_buf_count: Bytes of partial data in part_buf.
>>> @@ -154,6 +155,7 @@ struct dw_mci {
>>> spinlock_t irq_lock;
>>> void __iomem *regs;
>>> void __iomem *fifo_reg;
>>> + u32 fifo_addr_override;
>>>
>>> struct scatterlist *sg;
>>> struct sg_mapping_iter sg_miter;
>>>
>>
>
>
>
^ permalink raw reply
* RE: [v15, 3/7] powerpc/fsl: move mpc85xx.h to include/linux/fsl
From: Y.B. Lu @ 2016-10-31 9:35 UTC (permalink / raw)
To: Arnd Bergmann,
linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
Cc: Mark Rutland, ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, M.H. Lian,
linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-clk-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Qiang Zhao,
Russell King, Bhupesh Sharma, Claudiu Manoil,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Kumar Gala,
Scott Wood, Rob Herring, Santosh Shilimkar,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
Greg Kroah-Hartman,
linux-mmc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, X.B. Xie
In-Reply-To: <2723366.1bJeJ7SKI6@wuerfel>
> -----Original Message-----
> From: Arnd Bergmann [mailto:arnd-r2nGTMty4D4@public.gmane.org]
> Sent: Friday, October 28, 2016 6:53 PM
> To: linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
> Cc: Y.B. Lu; linux-mmc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org; Scott
> Wood; Mark Rutland; Greg Kroah-Hartman; X.B. Xie; M.H. Lian; linux-
> i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-clk-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Qiang Zhao; Russell King;
> Bhupesh Sharma; Joerg Roedel; Claudiu Manoil; devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org;
> Rob Herring; Santosh Shilimkar; linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org;
> netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Leo Li;
> iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org; Kumar Gala
> Subject: Re: [v15, 3/7] powerpc/fsl: move mpc85xx.h to include/linux/fsl
>
> On Friday, October 28, 2016 2:50:14 PM CEST Yangbo Lu wrote:
> > Move mpc85xx.h to include/linux/fsl and rename it to svr.h as a common
> > header file. This SVR numberspace is used on some ARM chips as well
> > as PPC, and even to check for a PPC SVR multi-arch drivers would
> > otherwise need to ifdef the header inclusion and all references to the
> SVR symbols.
> >
> >
>
> I don't see any of the contents of this header referenced by the soc
> driver any more. I think you can just drop this patch.
>
[Lu Yangbo-B47093] This header file was included by guts.c.
The guts driver used macro SVR_MAJ/SVR_MIN for calculation.
This header file was for powerpc arch before. And this patch is to made it as common header file for both ARM and PPC.
Sooner or later this is needed.
> Arnd
^ permalink raw reply
* Re: [PATCH v2 5/5] mmc: dw: Add fifo watermark quirk
From: Jaehoon Chung @ 2016-10-31 9:35 UTC (permalink / raw)
To: Jun Nie; +Cc: Shawn Guo, xie.baoyou, Ulf Hansson, Jason Liu, linux-mmc
In-Reply-To: <CABymUCMqjcFmK=T8Um+zLqECvzv+o4M8VjribrG=SrePn5BU+Q@mail.gmail.com>
On 10/31/2016 05:48 PM, Jun Nie wrote:
> 2016-10-28 13:30 GMT+08:00 Jaehoon Chung <jh80.chung@samsung.com>:
>> On 10/28/2016 11:37 AM, Jun Nie wrote:
>>> Data done irq is expected if data length is less than
>>> watermark in PIO mode. But fifo watermark is requested
>>> to be aligned with data length in some SoC so that TX/RX
>>> irq can be generated with data done irq. Add the
>>> watermark quirk to mark this requirement and force
>>> fifo watermark setting accordingly.
>>>
>>> Signed-off-by: Jun Nie <jun.nie@linaro.org>
>>> ---
>>> drivers/mmc/host/dw_mmc.c | 11 +++++++++--
>>> include/linux/mmc/dw_mmc.h | 2 ++
>>> 2 files changed, 11 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>>> index 24ae05b6..e0f49cc 100644
>>> --- a/drivers/mmc/host/dw_mmc.c
>>> +++ b/drivers/mmc/host/dw_mmc.c
>>> @@ -1111,11 +1111,15 @@ static void dw_mci_submit_data(struct dw_mci *host, struct mmc_data *data)
>>> mci_writel(host, CTRL, temp);
>>>
>>> /*
>>> - * Use the initial fifoth_val for PIO mode.
>>> + * Use the initial fifoth_val for PIO mode. If wm_quirk
>>> + * is set, we set watermark same as data size.
>>> * If next issued data may be transfered by DMA mode,
>>> * prev_blksz should be invalidated.
>>> */
>>> - mci_writel(host, FIFOTH, host->fifoth_val);
>>> + if (host->wm_quirk)
>>> + dw_mci_adjust_fifoth(host, data);
>>> + else
>>> + mci_writel(host, FIFOTH, host->fifoth_val);
>>> host->prev_blksz = 0;
>>> } else {
>>> /*
>>> @@ -2957,6 +2961,9 @@ static struct dw_mci_board *dw_mci_parse_dt(struct dw_mci *host)
>>>
>>> of_property_read_u32(np, "fifo-addr", &host->fifo_addr_override);
>>>
>>> + if (of_get_property(np, "fifo-watermark-quirk", NULL))
>>> + host->wm_quirk = true;
>>> +
>>> if (!of_property_read_u32(np, "clock-frequency", &clock_frequency))
>>> pdata->bus_hz = clock_frequency;
>>>
>>> diff --git a/include/linux/mmc/dw_mmc.h b/include/linux/mmc/dw_mmc.h
>>> index 4866ef5..2ccfd9c 100644
>>> --- a/include/linux/mmc/dw_mmc.h
>>> +++ b/include/linux/mmc/dw_mmc.h
>>> @@ -108,6 +108,7 @@ struct dw_mci_dma_slave {
>>> * @slot: Slots sharing this MMC controller.
>>> * @fifo_depth: depth of FIFO.
>>> * @fifo_addr_override: override fifo reg offset with this value.
>>> + * @wm_quirk: force fifo watermark equal with data length in PIO mode.
>>
>> quirk...hmm..It might be just my preference..
>> quirk looks like workaround...so how about changing other name?
>
> How about fifo_wm_aligned ?
If it's not quirk or workaround, whatever. :)
Best Regards,
Jaehoon Chung
>
>>
>>> * @data_shift: log2 of FIFO item size.
>>> * @part_buf_start: Start index in part_buf.
>>> * @part_buf_count: Bytes of partial data in part_buf.
>>> @@ -156,6 +157,7 @@ struct dw_mci {
>>> void __iomem *regs;
>>> void __iomem *fifo_reg;
>>> u32 fifo_addr_override;
>>> + u32 wm_quirk;
>>
>> For True or false, u32?
> True means alignment is needed. Will add this to comments.
>>
>>>
>>> struct scatterlist *sg;
>>> struct sg_mapping_iter sg_miter;
>>>
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
>
^ permalink raw reply
* RE: [v15, 6/7] base: soc: introduce soc_device_match() interface
From: Y.B. Lu @ 2016-10-31 9:28 UTC (permalink / raw)
To: Arnd Bergmann,
linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
Cc: Mark Rutland, ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org,
Geert Uytterhoeven,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, M.H. Lian,
linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-clk-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Qiang Zhao,
Russell King, Bhupesh Sharma, Claudiu Manoil,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Kumar Gala,
Scott Wood, Rob Herring, Santosh Shilimkar,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
Greg Kroah-Hartman,
"linux-mmc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" <linux-mmc>
In-Reply-To: <2572890.e6aV4hmMEL@wuerfel>
> -----Original Message-----
> From: Arnd Bergmann [mailto:arnd-r2nGTMty4D4@public.gmane.org]
> Sent: Friday, October 28, 2016 6:48 PM
> To: linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
> Cc: Y.B. Lu; linux-mmc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org; Scott
> Wood; Mark Rutland; Greg Kroah-Hartman; X.B. Xie; M.H. Lian; linux-
> i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-clk-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Qiang Zhao; Russell King;
> Bhupesh Sharma; Joerg Roedel; Claudiu Manoil; devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org;
> Rob Herring; Santosh Shilimkar; linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org;
> netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Leo Li;
> iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org; Kumar Gala; Geert Uytterhoeven
> Subject: Re: [v15, 6/7] base: soc: introduce soc_device_match() interface
>
> On Friday, October 28, 2016 2:50:17 PM CEST Yangbo Lu wrote:
> > +
> > +static int soc_device_match_one(struct device *dev, void *arg) {
> > + struct soc_device *soc_dev = container_of(dev, struct
> soc_device, dev);
> > + const struct soc_device_attribute *match = arg;
> > +
> > + if (match->machine &&
> > + !glob_match(match->machine, soc_dev->attr->machine))
> > + return 0;
> > +
> > + if (match->family &&
> > + !glob_match(match->family, soc_dev->attr->family))
> > + return 0;
> > +
> >
>
> Geert found a bug in my code, and submitted a fix at
> https://patchwork.kernel.org/patch/9361395/
>
> I think you should include that one in your series.
>
[Lu Yangbo-B47093] Ok, no problem. Thanks :)
> Arnd
^ permalink raw reply
* Re: [PATCH v2 4/5] mmc: dw: Add fifo address property
From: Jun Nie @ 2016-10-31 8:50 UTC (permalink / raw)
To: Jaehoon Chung; +Cc: Shawn Guo, xie.baoyou, Ulf Hansson, Jason Liu, linux-mmc
In-Reply-To: <bc49c0a3-5159-03a2-0023-e32f2dbbf956@samsung.com>
2016-10-28 13:24 GMT+08:00 Jaehoon Chung <jh80.chung@samsung.com>:
> On 10/28/2016 11:37 AM, Jun Nie wrote:
>> The FIFO address may break default address assumption of 0x100
>> (version < 0x240A) and 0x200(version >= 0x240A) in current driver.
>> The new property is introduced to override fifo address via DT
>> node information.
>>
>> Signed-off-by: Jun Nie <jun.nie@linaro.org>
>> ---
>> drivers/mmc/host/dw_mmc.c | 5 +++++
>> include/linux/mmc/dw_mmc.h | 2 ++
>> 2 files changed, 7 insertions(+)
>>
>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>> index 1c9ee36..24ae05b6 100644
>> --- a/drivers/mmc/host/dw_mmc.c
>> +++ b/drivers/mmc/host/dw_mmc.c
>> @@ -2955,6 +2955,8 @@ static struct dw_mci_board *dw_mci_parse_dt(struct dw_mci *host)
>>
>> of_property_read_u32(np, "card-detect-delay", &pdata->detect_delay_ms);
>>
>> + of_property_read_u32(np, "fifo-addr", &host->fifo_addr_override);
>> +
>> if (!of_property_read_u32(np, "clock-frequency", &clock_frequency))
>> pdata->bus_hz = clock_frequency;
>>
>> @@ -3163,6 +3165,9 @@ int dw_mci_probe(struct dw_mci *host)
>> else
>> host->fifo_reg = host->regs + DATA_240A_OFFSET;
>>
>> + if (host->fifo_addr_override)
>> + host->fifo_reg = host->regs + host->fifo_addr_override;
>> +
>
> Check condition the sequentially.
>
> if (host->fifo_addr_override) {
> ...
> } else if (host->verid < DW_MMC_240A) {
> ..
> } else {
> ..
> }
>
> how about?
Okay, will change to this.
>
>> tasklet_init(&host->tasklet, dw_mci_tasklet_func, (unsigned long)host);
>> ret = devm_request_irq(host->dev, host->irq, dw_mci_interrupt,
>> host->irq_flags, "dw-mci", host);
>> diff --git a/include/linux/mmc/dw_mmc.h b/include/linux/mmc/dw_mmc.h
>> index f5af2bd..4866ef5 100644
>> --- a/include/linux/mmc/dw_mmc.h
>> +++ b/include/linux/mmc/dw_mmc.h
>> @@ -107,6 +107,7 @@ struct dw_mci_dma_slave {
>> * @ciu_clk: Pointer to card interface unit clock instance.
>> * @slot: Slots sharing this MMC controller.
>> * @fifo_depth: depth of FIFO.
>> + * @fifo_addr_override: override fifo reg offset with this value.
>
> DATA addr is more correct. it's related with DATA register.
You name it, will change to data_addr_override.
>
> Best Regards,
> Jaehoon Chung
>
>> * @data_shift: log2 of FIFO item size.
>> * @part_buf_start: Start index in part_buf.
>> * @part_buf_count: Bytes of partial data in part_buf.
>> @@ -154,6 +155,7 @@ struct dw_mci {
>> spinlock_t irq_lock;
>> void __iomem *regs;
>> void __iomem *fifo_reg;
>> + u32 fifo_addr_override;
>>
>> struct scatterlist *sg;
>> struct sg_mapping_iter sg_miter;
>>
>
^ permalink raw reply
* Re: [PATCH v2 5/5] mmc: dw: Add fifo watermark quirk
From: Jun Nie @ 2016-10-31 8:48 UTC (permalink / raw)
To: Jaehoon Chung; +Cc: Shawn Guo, xie.baoyou, Ulf Hansson, Jason Liu, linux-mmc
In-Reply-To: <739e77cb-c873-569c-a812-24fa752b41b0@samsung.com>
2016-10-28 13:30 GMT+08:00 Jaehoon Chung <jh80.chung@samsung.com>:
> On 10/28/2016 11:37 AM, Jun Nie wrote:
>> Data done irq is expected if data length is less than
>> watermark in PIO mode. But fifo watermark is requested
>> to be aligned with data length in some SoC so that TX/RX
>> irq can be generated with data done irq. Add the
>> watermark quirk to mark this requirement and force
>> fifo watermark setting accordingly.
>>
>> Signed-off-by: Jun Nie <jun.nie@linaro.org>
>> ---
>> drivers/mmc/host/dw_mmc.c | 11 +++++++++--
>> include/linux/mmc/dw_mmc.h | 2 ++
>> 2 files changed, 11 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>> index 24ae05b6..e0f49cc 100644
>> --- a/drivers/mmc/host/dw_mmc.c
>> +++ b/drivers/mmc/host/dw_mmc.c
>> @@ -1111,11 +1111,15 @@ static void dw_mci_submit_data(struct dw_mci *host, struct mmc_data *data)
>> mci_writel(host, CTRL, temp);
>>
>> /*
>> - * Use the initial fifoth_val for PIO mode.
>> + * Use the initial fifoth_val for PIO mode. If wm_quirk
>> + * is set, we set watermark same as data size.
>> * If next issued data may be transfered by DMA mode,
>> * prev_blksz should be invalidated.
>> */
>> - mci_writel(host, FIFOTH, host->fifoth_val);
>> + if (host->wm_quirk)
>> + dw_mci_adjust_fifoth(host, data);
>> + else
>> + mci_writel(host, FIFOTH, host->fifoth_val);
>> host->prev_blksz = 0;
>> } else {
>> /*
>> @@ -2957,6 +2961,9 @@ static struct dw_mci_board *dw_mci_parse_dt(struct dw_mci *host)
>>
>> of_property_read_u32(np, "fifo-addr", &host->fifo_addr_override);
>>
>> + if (of_get_property(np, "fifo-watermark-quirk", NULL))
>> + host->wm_quirk = true;
>> +
>> if (!of_property_read_u32(np, "clock-frequency", &clock_frequency))
>> pdata->bus_hz = clock_frequency;
>>
>> diff --git a/include/linux/mmc/dw_mmc.h b/include/linux/mmc/dw_mmc.h
>> index 4866ef5..2ccfd9c 100644
>> --- a/include/linux/mmc/dw_mmc.h
>> +++ b/include/linux/mmc/dw_mmc.h
>> @@ -108,6 +108,7 @@ struct dw_mci_dma_slave {
>> * @slot: Slots sharing this MMC controller.
>> * @fifo_depth: depth of FIFO.
>> * @fifo_addr_override: override fifo reg offset with this value.
>> + * @wm_quirk: force fifo watermark equal with data length in PIO mode.
>
> quirk...hmm..It might be just my preference..
> quirk looks like workaround...so how about changing other name?
How about fifo_wm_aligned ?
>
>> * @data_shift: log2 of FIFO item size.
>> * @part_buf_start: Start index in part_buf.
>> * @part_buf_count: Bytes of partial data in part_buf.
>> @@ -156,6 +157,7 @@ struct dw_mci {
>> void __iomem *regs;
>> void __iomem *fifo_reg;
>> u32 fifo_addr_override;
>> + u32 wm_quirk;
>
> For True or false, u32?
True means alignment is needed. Will add this to comments.
>
>>
>> struct scatterlist *sg;
>> struct sg_mapping_iter sg_miter;
>>
>
^ permalink raw reply
* Re: [PATCH v2 2/5] mmc: zx: Initial support for ZX mmc controller
From: Jun Nie @ 2016-10-31 8:47 UTC (permalink / raw)
To: Jaehoon Chung; +Cc: Shawn Guo, xie.baoyou, Ulf Hansson, Jason Liu, linux-mmc
In-Reply-To: <331dab06-72d2-d382-f46b-04c2a85b70d1@samsung.com>
2016-10-28 13:16 GMT+08:00 Jaehoon Chung <jh80.chung@samsung.com>:
> On 10/28/2016 11:37 AM, Jun Nie wrote:
>> This platform driver adds initial support for the DW host controller
>> found on ZTE SoCs.
>>
>> It has been tested on ZX296718 EVB board currently. More support on
>> timing tuning will be added when hardware is available.
>>
>> Signed-off-by: Jun Nie <jun.nie@linaro.org>
>> ---
>> drivers/mmc/host/Kconfig | 9 ++
>> drivers/mmc/host/Makefile | 1 +
>> drivers/mmc/host/dw_mmc-zx.c | 230 +++++++++++++++++++++++++++++++++++++++++++
>> drivers/mmc/host/dw_mmc-zx.h | 23 +++++
>> 4 files changed, 263 insertions(+)
>> create mode 100644 drivers/mmc/host/dw_mmc-zx.c
>> create mode 100644 drivers/mmc/host/dw_mmc-zx.h
>>
>> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
>> index 5274f50..2b3202c 100644
>> --- a/drivers/mmc/host/Kconfig
>> +++ b/drivers/mmc/host/Kconfig
>> @@ -662,6 +662,15 @@ config MMC_DW_ROCKCHIP
>> Synopsys DesignWare Memory Card Interface driver. Select this option
>> for platforms based on RK3066, RK3188 and RK3288 SoC's.
>>
>> +config MMC_DW_ZX
>> + tristate "ZTE specific extensions for Synopsys DW Memory Card Interface"
>> + depends on MMC_DW
>
> I guess MMC_DW_ZX depends on your SoC config, doesn't?
Right, will add dependency.
>
>> + select MMC_DW_PLTFM
>> + help
>> + This selects support for ZTE SoC specific extensions to the
>> + Synopsys DesignWare Memory Card Interface driver. Select this option
>> + for platforms based on ZX296718 SoC's.
>> +
>> config MMC_SH_MMCIF
>> tristate "SuperH Internal MMCIF support"
>> depends on HAS_DMA
>> diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
>> index e2bdaaf..9766143 100644
>> --- a/drivers/mmc/host/Makefile
>> +++ b/drivers/mmc/host/Makefile
>> @@ -48,6 +48,7 @@ obj-$(CONFIG_MMC_DW_EXYNOS) += dw_mmc-exynos.o
>> obj-$(CONFIG_MMC_DW_K3) += dw_mmc-k3.o
>> obj-$(CONFIG_MMC_DW_PCI) += dw_mmc-pci.o
>> obj-$(CONFIG_MMC_DW_ROCKCHIP) += dw_mmc-rockchip.o
>> +obj-$(CONFIG_MMC_DW_ZX) += dw_mmc-zx.o
>> obj-$(CONFIG_MMC_SH_MMCIF) += sh_mmcif.o
>> obj-$(CONFIG_MMC_JZ4740) += jz4740_mmc.o
>> obj-$(CONFIG_MMC_VUB300) += vub300.o
>> diff --git a/drivers/mmc/host/dw_mmc-zx.c b/drivers/mmc/host/dw_mmc-zx.c
>> new file mode 100644
>> index 0000000..0404f8e
>> --- /dev/null
>> +++ b/drivers/mmc/host/dw_mmc-zx.c
>> @@ -0,0 +1,230 @@
>> +/*
>> + * ZX Specific Extensions for Synopsys DW Multimedia Card Interface driver
>> + *
>> + * Copyright (C) 2016, Linaro Ltd.
>> + * Copyright (C) 2016, ZTE Corp.
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + */
>> +
>> +#include <linux/clk.h>
>> +#include <linux/mfd/syscon.h>
>> +#include <linux/mmc/dw_mmc.h>
>> +#include <linux/mmc/host.h>
>> +#include <linux/mmc/mmc.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/pm_runtime.h>
>> +#include <linux/regmap.h>
>> +#include <linux/slab.h>
>> +
>> +#include "dw_mmc.h"
>> +#include "dw_mmc-pltfm.h"
>> +#include "dw_mmc-zx.h"
>> +
>> +#define ZX_DLL_LOCKED BIT(2)
>
> Some DLL rigsters and bits are defined in dw_mmc-zx.h.
> why defined ZX_DLL_LOCKED at here.
>
> You can choose that all defines locates to dw_mmc-zx.c or dw_mmc-zx.h
>
Will move together.
>> +
>> +struct dw_mci_zx_priv_data {
>> + struct regmap *sysc_base;
>> +};
>> +
>> +static int dw_mci_zx_emmc_set_delay(struct dw_mci *host, unsigned int delay,
>> + unsigned int clk_flag)
>
> Why do you use "unsigned int" as clk_flag? It's just use one of 0 and 1.
> And 0 and 1 are what means? On/Off?
>
Yes, enumeration shall looks better here. It is a flag for different
delay type. Will change to enumeration.
>> +{
>> + struct dw_mci_zx_priv_data *priv = host->priv;
>> + struct regmap *sysc_base = priv->sysc_base;
>> + unsigned int clksel;
>> + unsigned int loop = 1000;
>> + int ret;
>> +
>
> priv->sysc_base doesn't never NULL?
For this SoC, it is never NULL if dts is correct. Adding a check is
better anyway.
>
>> + ret = regmap_write(sysc_base, LB_AON_EMMC_CFG_REG0,
>> + PARA_DLL_START_POINT(4) | PARA_DLL_LOCK_NUM(4));
>
> Could you add the comment for controlling this regs?
> I'm not sure because i didn't have ZX TRM..but PARA_DLL_LOCK_NUM should be locked?
>
> It doesn't affect to other bit?
> I think you can use the regmap_update_bits instead of regmap_write.
It does not affect other bit as all bits are write as desired. But
your suggestion make it clearer.
>
>> + if (ret)
>> + return ret;
>> +
>> + ret = regmap_read(sysc_base, LB_AON_EMMC_CFG_REG1, &clksel);
>> + if (ret)
>> + return ret;
>> +
>> + if (clk_flag) {
>> + clksel &= ~(CLK_SAMP_DELAY(0x7F));
>
> It's meaningless..CLK_SAMP_DELAY used only at here.
> CLK_SAMP_DELAY ((x) & 0x7F << 0)
>
> It's just CLK_SAMP_DELAY_MASK.?
>
> #define CLK_SAMP_DELAY_MASK (0x7F << 0)
> clksel &= ~CLK_SAMP_DELAY_MASK;
>
Right, clearer.
>
>> + clksel |= (delay << 8);
>
> Use the CLK_SAMP_DELAY_SHIFT instead of 8.
>
>> + } else {
>> + clksel &= ~(READ_DQS_DELAY(0x7F));
>
> Ditto.
> And i think it also can be changed to regmap_update_bits.
>
>> + clksel |= delay;
>> + }
>> +
>> + regmap_write(sysc_base, LB_AON_EMMC_CFG_REG1, clksel);
>> + regmap_write(sysc_base, LB_AON_EMMC_CFG_REG0,
>> + PARA_DLL_START_POINT(4) | PARA_DLL_LOCK_NUM(4) |
>> + DLL_REG_SET);
>
> regmap_update_bits?
Will do.
>
>> +
>> + do {
>> + ret = regmap_read(sysc_base, LB_AON_EMMC_CFG_REG2, &clksel);
>> + if (ret)
>> + return ret;
>> +
>> + } while (--loop && !(clksel & ZX_DLL_LOCKED));
>> +
>> + if (!loop) {
>> + dev_err(host->dev, "Error: %s dll lock fail\n", __func__);
>> + return -EIO;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int dw_mci_zx_emmc_execute_tuning(struct dw_mci_slot *slot, u32 opcode)
>> +{
>> + struct dw_mci *host = slot->host;
>> + struct mmc_host *mmc = slot->mmc;
>> + int len, start = 0, end = 0, delay, best = 0;
>> + int ret = 0;
>> +
>> + for (delay = 1 ; delay < 128; delay++) {
>> + ret = dw_mci_zx_emmc_set_delay(host, delay, 1);
>> + if (ret)
>> + return ret;
>
> When it's failed, just returned.
> Doesn't need to try with next delay value?
Yes, more retry make robust.
>
>> +
>> + if (mmc_send_tuning(mmc, opcode, NULL)) {
>> + if (start >= 0) {
>> + end = delay - 1;
>> + /* check and update longest good range */
>> + if ((end - start) > len) {
>> + best = (start + end) >> 1;
>> + len = end - start;
>> + }
>> + }
>> + start = -1;
>> + end = 0;
>> + continue;
>> + }
>> + if (start < 0)
>> + start = delay;
>> + }
>> +
>> + if (start >= 0) {
>> + end = delay - 1;
>> + if ((end - start) > len) {
>> + best = (start + end) >> 1;
>> + len = end - start;
>> + }
>> + }
>> + if (best < 0)
>> + return -EIO;
>> +
>> + dev_info(host->dev, "%s best range: start %d end %d\n", __func__,
>> + start, end);
>> + dw_mci_zx_emmc_set_delay(host, best, 1);
>> + return 0;
>> +}
>> +
>> +static int dw_mci_zx_prepare_hs400_tuning(struct dw_mci *host,
>> + struct mmc_ios *ios)
>> +{
>> + int ret;
>> +
>> + /* config phase shift 90 */
>> + ret = dw_mci_zx_emmc_set_delay(host, 32, 0);
>
> It's always fixed to 32? What means 32?
It means 90 degree shift for value 32. This configuration comes from
ZTE engineer as I do not have hardware for this tuning. Let's just
keep it with adding more comments till tuning is needed.
>
>> + if (ret < 0)
>> + return -EIO;
>> +
>> + return 0;
>> +}
>> +
>> +static int dw_mci_zx_execute_tuning(struct dw_mci_slot *slot, u32 opcode)
>> +{
>> + struct dw_mci *host = slot->host;
>> +
>> + if (host->verid == 0x290a) /* emmc */
>> + return dw_mci_zx_emmc_execute_tuning(slot, opcode);
>
> I didn't know why you check host->verid is 2.90a..
> Is there any reason?
There are two version DW MMC IP on this SoC and different
configuration is needed for them. I do not have hardware for tuning
version 210A timing and will be added later.
>
>> +
>> + return 0;
>> +}
>> +
>> +static int dw_mci_zx_parse_dt(struct dw_mci *host)
>> +{
>> + struct device_node *np = host->dev->of_node;
>> + struct device_node *node;
>> + struct dw_mci_zx_priv_data *priv;
>> + struct regmap *sysc_base;
>> + int ret;
>> +
>> + node = of_parse_phandle(np, "zte,aon-syscon", 0);
>> + if (node) {
>> + sysc_base = syscon_node_to_regmap(node);
>> + of_node_put(node);
>
> Use the syscon_regmap_lookup_by_phandle(). It's same behavior.
Will do.
>
>> +
>> + if (IS_ERR(sysc_base)) {
>> + ret = PTR_ERR(sysc_base);
>> + if (ret != -EPROBE_DEFER)
>> + dev_err(host->dev, "Can't get syscon: %d\n",
>> + ret);
>> + return ret;
>> + }
>> + }
>> +
>> + priv = devm_kzalloc(host->dev, sizeof(*priv), GFP_KERNEL);
>> + if (!priv)
>> + return -ENOMEM;
>> + priv->sysc_base = sysc_base;
>
> Is there no case that sysc_base is NULL?
sysc_base is needed only for eMMC. So it is NULL for SD/MMC cases, and
we can save memory for SD/MMC cases here :)
>
>> + host->priv = priv;
>> +
>> + return 0;
>> +}
>> +
>> +static unsigned long zx_dwmmc_caps[3] = {
>> + MMC_CAP_CMD23,
>> + MMC_CAP_CMD23,
>> + MMC_CAP_CMD23,
>> +};
>> +
>> +static const struct dw_mci_drv_data zx_drv_data = {
>> + .caps = zx_dwmmc_caps,
>> + .execute_tuning = dw_mci_zx_execute_tuning,
>> + .prepare_hs400_tuning = dw_mci_zx_prepare_hs400_tuning,
>> + .parse_dt = dw_mci_zx_parse_dt,
>> +};
>> +
>> +static const struct of_device_id dw_mci_zx_match[] = {
>> + { .compatible = "zte,dw-mshc", .data = &zx_drv_data},
>> +};
>> +MODULE_DEVICE_TABLE(of, dw_mci_zx_match);
>> +
>> +static int dw_mci_zx_probe(struct platform_device *pdev)
>> +{
>> + const struct dw_mci_drv_data *drv_data;
>> + const struct of_device_id *match;
>> +
>> + match = of_match_node(dw_mci_zx_match, pdev->dev.of_node);
>> + drv_data = match->data;
>> +
>> + return dw_mci_pltfm_register(pdev, drv_data);
>> +}
>> +
>> +static const struct dev_pm_ops dw_mci_zx_dev_pm_ops = {
>> + SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
>> + pm_runtime_force_resume)
>> + SET_RUNTIME_PM_OPS(dw_mci_runtime_suspend,
>> + dw_mci_runtime_resume,
>> + NULL)
>> +};
>> +
>> +static struct platform_driver dw_mci_zx_pltfm_driver = {
>> + .probe = dw_mci_zx_probe,
>> + .remove = dw_mci_pltfm_remove,
>> + .driver = {
>> + .name = "dwmmc_zx",
>> + .of_match_table = dw_mci_zx_match,
>> + .pm = &dw_mci_zx_dev_pm_ops,
>> + },
>> +};
>> +
>> +module_platform_driver(dw_mci_zx_pltfm_driver);
>> +
>> +MODULE_DESCRIPTION("ZTE emmc/sd driver");
>> +MODULE_LICENSE("GPL v2");
>> diff --git a/drivers/mmc/host/dw_mmc-zx.h b/drivers/mmc/host/dw_mmc-zx.h
>> new file mode 100644
>> index 0000000..b1aac52
>> --- /dev/null
>> +++ b/drivers/mmc/host/dw_mmc-zx.h
>> @@ -0,0 +1,23 @@
>> +#ifndef _DW_MMC_ZX_H_
>> +#define _DW_MMC_ZX_H_
>> +
>> +/* dll reg offset*/
>> +#define LB_AON_EMMC_CFG_REG0 0x1B0
>> +#define LB_AON_EMMC_CFG_REG1 0x1B4
>> +#define LB_AON_EMMC_CFG_REG2 0x1B8
>> +
>> +/* LB_AON_EMMC_CFG_REG0 register defines */
>> +#define PARA_DLL_START_POINT(x) (((x) & 0xFF) << 0)
>> +#define DLL_REG_SET BIT(8)
>> +#define PARA_DLL_LOCK_NUM(x) (((x) & 7) << 16)
>> +#define PARA_PHASE_DET_SEL(x) (((x) & 7) << 20)
>> +#define PARA_DLL_BYPASS_MODE BIT(23)
>> +#define PARA_HALF_CLK_MODE BIT(24)
>
> PAR_PHASE_DET_SEL/PARA_DLL_BYPASS_MODE_BIT/PARA_HALF_CLK_MODE are never used anywhere.
>
>> +
>> +/* LB_AON_EMMC_CFG_REG1 register defines */
>> +#define READ_DQS_DELAY(x) (((x) & 0x7F) << 0)
>> +#define READ_DQS_BYPASS_MODE BIT(7)
>> +#define CLK_SAMP_DELAY(x) (((x) & 0x7F) << 8)
>> +#define CLK_SAMP_BYPASS_MODE BIT(15)
>
> Also READ_DQS_BYPASS_MODe/CLK_SAMP_BYBASS_MODE didnt used anywhere.
>
>
> Hmm. These are not dwmmc host controller's register.
> So If you needs to add these defines..I think you needs to add dessriptions in more detail.
>
> At least..Which board's DLL reg offset.
>
> Best Regards,
> Jaehoon Chung
>
>> +
>> +#endif /* _DW_MMC_ZX_H_ */
>>
>
^ permalink raw reply
* Re: [PATCH 1/2] mmc: sdhci: dt: Add device tree properties sdhci-caps and sdhci-caps-mask
From: Rob Herring @ 2016-10-31 6:08 UTC (permalink / raw)
To: Zach Brown
Cc: ulf.hansson, adrian.hunter, mark.rutland, linux-mmc, devicetree,
linux-kernel
In-Reply-To: <1477670171-30015-2-git-send-email-zach.brown@ni.com>
On Fri, Oct 28, 2016 at 10:56:10AM -0500, Zach Brown wrote:
> On some systems the sdhci capabilty register is incorrect for one
> reason or another.
>
> The sdhci-caps-mask property specifies which bits in the register
> are incorrect and should be turned off before using sdhci-caps to turn
> on bits.
>
> The sdhci-caps property specifies which bits should be turned on.
>
> Signed-off-by: Zach Brown <zach.brown@ni.com>
> ---
> Documentation/devicetree/bindings/mmc/sdhci.txt | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mmc/sdhci.txt
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* [PATCH 2/2] mmc: dw_mmc: add the "reset" as name of reset controller
From: Jaehoon Chung @ 2016-10-31 2:49 UTC (permalink / raw)
To: linux-mmc, devicetree
Cc: ulf.hansson, robh+dt, mark.rutland, linux-kernel, john.stultz,
guodong.xu, leo.yan, vincent.guittot, Jaehoon Chung
In-Reply-To: <20161031024942.4415-1-jh80.chung@samsung.com>
Add the "reset" as name of reset controller.
This is for preventing the wrong operation. Even if some SoC has reset
controller, doesn't define "resets" in device-tree.
Then it might be waiting for reset controller and it should be stuck.
Fixes: d6786fefe816 ("mmc: dw_mmc: add reset support to dwmmc host controller")
Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
---
drivers/mmc/host/dw_mmc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 1c9ee36..a16c537 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -2940,7 +2940,7 @@ static struct dw_mci_board *dw_mci_parse_dt(struct dw_mci *host)
return ERR_PTR(-ENOMEM);
/* find reset controller when exist */
- pdata->rstc = devm_reset_control_get_optional(dev, NULL);
+ pdata->rstc = devm_reset_control_get_optional(dev, "reset");
if (IS_ERR(pdata->rstc)) {
if (PTR_ERR(pdata->rstc) == -EPROBE_DEFER)
return ERR_PTR(-EPROBE_DEFER);
--
2.10.1
^ permalink raw reply related
* [PATCH 1/2] Documentation: synopsys-dw-mshc: add binding for reset-names
From: Jaehoon Chung @ 2016-10-31 2:49 UTC (permalink / raw)
To: linux-mmc-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
Cc: ulf.hansson-QSEj5FYQhm4dnm+yROfE0A,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
john.stultz-QSEj5FYQhm4dnm+yROfE0A,
guodong.xu-QSEj5FYQhm4dnm+yROfE0A, leo.yan-QSEj5FYQhm4dnm+yROfE0A,
vincent.guittot-QSEj5FYQhm4dnm+yROfE0A, Jaehoon Chung
In-Reply-To: <20161031024942.4415-1-jh80.chung-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
Add reset-names property for binding dw-mmc controller.
It might be used together with "reset" property.
- Note: It must be "reset" as name.
Fixes: d6786fefe816 ("mmc: dw_mmc: add reset support to dwmmc host controller")
Signed-off-by: Jaehoon Chung <jh80.chung-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
---
Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt b/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt
index 4e00e85..bfa461a 100644
--- a/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt
+++ b/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt
@@ -43,6 +43,9 @@ Optional properties:
reset signal present internally in some host controller IC designs.
See Documentation/devicetree/bindings/reset/reset.txt for details.
+* reset-names: request name for using "resets" property. Must be "reset".
+ (It will be used together with "resets" property.)
+
* clocks: from common clock binding: handle to biu and ciu clocks for the
bus interface unit clock and the card interface unit clock.
@@ -103,6 +106,8 @@ board specific portions as listed below.
interrupts = <0 75 0>;
#address-cells = <1>;
#size-cells = <0>;
+ resets = <&rst 20>;
+ reset-names = "reset";
};
[board specific internal DMA resources]
--
2.10.1
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH 0/2] mmc: dw_mmc: fix the wrong operation for reset controller
From: Jaehoon Chung @ 2016-10-31 2:49 UTC (permalink / raw)
To: linux-mmc, devicetree
Cc: ulf.hansson, robh+dt, mark.rutland, linux-kernel, john.stultz,
guodong.xu, leo.yan, vincent.guittot, Jaehoon Chung
This patch adds the "reset-names" as reset controller for dwmmc controller.
Linaro guys had reported the issue about booting stuck.
Some SoCs are enabled the CONFIG_RESET_CONTROLLER.
then dwmmc controller are waiting for getting reset controller.
But if doesn't define "resets" property in device-tree, it should be stuck.
If use the reset-names as reset controller for dwmmc controller,
it's more stable than now.
This commit is related with the below commit.
Fixes: d6786fefe816 ("mmc: dw_mmc: add reset support to dwmmc host controller")
Jaehoon Chung (2):
Documentation: synopsys-dw-mshc: add binding for reset-names
mmc: dw_mmc: add the "reset" as name of reset controller
Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt | 5 +++++
drivers/mmc/host/dw_mmc.c | 2 +-
2 files changed, 6 insertions(+), 1 deletion(-)
--
2.10.1
^ permalink raw reply
* [PATCH 2/2] mmc: sdhci: Use sdhci-caps-mask and sdhci-caps to change the caps read during __sdhci_read_caps
From: Zach Brown @ 2016-10-28 15:56 UTC (permalink / raw)
To: ulf.hansson
Cc: adrian.hunter, robh+dt, mark.rutland, linux-mmc, devicetree,
linux-kernel, zach.brown
In-Reply-To: <1477670171-30015-1-git-send-email-zach.brown@ni.com>
The sdhci capabilities register can be incorrect. The sdhci-caps-mask
and sdhci-caps dt properties specify which bits of the register are
incorrect and what their values should be. This patch makes the sdhci
driver use those properties to correct the caps during
__sdhci_read_caps.
During __sdhci_read_caps
Use the sdhci-caps-mask property to turn off the incorrect bits of the
sdhci register after reading them.
Use the sdhci-caps to turn on bits after using sdhci-caps-mask to turn
off the incorrect ones.
Signed-off-by: Zach Brown <zach.brown@ni.com>
---
drivers/mmc/host/sdhci.c | 24 ++++++++++++++++++++++--
1 file changed, 22 insertions(+), 2 deletions(-)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 1e25b01..d5feae4 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -22,6 +22,7 @@
#include <linux/scatterlist.h>
#include <linux/regulator/consumer.h>
#include <linux/pm_runtime.h>
+#include <linux/of.h>
#include <linux/leds.h>
@@ -2991,6 +2992,8 @@ static int sdhci_set_dma_mask(struct sdhci_host *host)
void __sdhci_read_caps(struct sdhci_host *host, u16 *ver, u32 *caps, u32 *caps1)
{
u16 v;
+ u64 dt_caps_mask = 0;
+ u64 dt_caps = 0;
if (host->read_caps)
return;
@@ -3005,18 +3008,35 @@ void __sdhci_read_caps(struct sdhci_host *host, u16 *ver, u32 *caps, u32 *caps1)
sdhci_do_reset(host, SDHCI_RESET_ALL);
+ of_property_read_u64(mmc_dev(host->mmc)->of_node,
+ "sdhci-caps-mask", &dt_caps_mask);
+ of_property_read_u64(mmc_dev(host->mmc)->of_node,
+ "sdhci-caps", &dt_caps);
+
v = ver ? *ver : sdhci_readw(host, SDHCI_HOST_VERSION);
host->version = (v & SDHCI_SPEC_VER_MASK) >> SDHCI_SPEC_VER_SHIFT;
if (host->quirks & SDHCI_QUIRK_MISSING_CAPS)
return;
- host->caps = caps ? *caps : sdhci_readl(host, SDHCI_CAPABILITIES);
+ if (caps)
+ host->caps = *caps;
+ else {
+ host->caps = sdhci_readl(host, SDHCI_CAPABILITIES);
+ host->caps &= ~lower_32_bits(dt_caps_mask);
+ host->caps |= lower_32_bits(dt_caps);
+ }
if (host->version < SDHCI_SPEC_300)
return;
- host->caps1 = caps1 ? *caps1 : sdhci_readl(host, SDHCI_CAPABILITIES_1);
+ if (caps1)
+ host->caps1 = *caps1;
+ else {
+ host->caps1 = sdhci_readl(host, SDHCI_CAPABILITIES_1);
+ host->caps1 &= ~upper_32_bits(dt_caps_mask);
+ host->caps1 |= upper_32_bits(dt_caps);
+ }
}
EXPORT_SYMBOL_GPL(__sdhci_read_caps);
--
2.7.4
^ permalink raw reply related
* [PATCH 1/2] mmc: sdhci: dt: Add device tree properties sdhci-caps and sdhci-caps-mask
From: Zach Brown @ 2016-10-28 15:56 UTC (permalink / raw)
To: ulf.hansson
Cc: adrian.hunter, robh+dt, mark.rutland, linux-mmc, devicetree,
linux-kernel, zach.brown
In-Reply-To: <1477670171-30015-1-git-send-email-zach.brown@ni.com>
On some systems the sdhci capabilty register is incorrect for one
reason or another.
The sdhci-caps-mask property specifies which bits in the register
are incorrect and should be turned off before using sdhci-caps to turn
on bits.
The sdhci-caps property specifies which bits should be turned on.
Signed-off-by: Zach Brown <zach.brown@ni.com>
---
Documentation/devicetree/bindings/mmc/sdhci.txt | 14 ++++++++++++++
1 file changed, 14 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mmc/sdhci.txt
diff --git a/Documentation/devicetree/bindings/mmc/sdhci.txt b/Documentation/devicetree/bindings/mmc/sdhci.txt
new file mode 100644
index 0000000..0f4f1bb
--- /dev/null
+++ b/Documentation/devicetree/bindings/mmc/sdhci.txt
@@ -0,0 +1,14 @@
+The properties specific for SD host controllers. For properties shared by MMC
+host controllers refer to the mmc[1] bindings.
+
+ [1] Documentation/devicetree/bindings/mmc/mmc.txt
+
+Optional properties:
+- sdhci-caps-mask: The sdhci capabilities register is incorrect. This 64bit
+ property corresponds to the bits in the sdhci capabilty register. If the bit
+ is on in the mask then the bit is incorrect in the register and should be
+ turned off, before applying sdhci-caps.
+- sdhci-caps: The sdhci capabilities register is incorrect. This 64bit
+ property corresponds to the bits in the sdhci capability register. If the
+ bit is on in the property then the bit should be turned on.
+
--
2.7.4
^ permalink raw reply related
* [PATCH 0/2] mmc: sdhci: Fix sdhci caps register bits with corrections provided by dt
From: Zach Brown @ 2016-10-28 15:56 UTC (permalink / raw)
To: ulf.hansson
Cc: adrian.hunter, robh+dt, mark.rutland, linux-mmc, devicetree,
linux-kernel, zach.brown
For various reasons the sdhci caps register can be incorrect. This patch set
introduces a general way to correct the bits when they are read to accurately
reflect the capabilties of the controller/board combo.
The first patch creates sdhci-caps and sdhci-caps-mask dt properties that
combined represent the correction to the sdhci caps register.
The second patch uses the new dt properties to correct the caps from the
register as they read during __sdhci_read_caps.
Changes from RFC:
* /s/registers/register
* Moved sdhci dt properties into new documentation file sdhci.txt
Zach Brown (2):
mmc: sdhci: dt: Add device tree properties sdhci-caps and
sdhci-caps-mask
mmc: sdhci: Use sdhci-caps-mask and sdhci-caps to change the caps read
during __sdhci_read_caps
Documentation/devicetree/bindings/mmc/sdhci.txt | 14 ++++++++++++++
drivers/mmc/host/sdhci.c | 24 ++++++++++++++++++++++--
2 files changed, 36 insertions(+), 2 deletions(-)
create mode 100644 Documentation/devicetree/bindings/mmc/sdhci.txt
--
2.7.4
^ permalink raw reply
* Re: [PATCH 1/2] mmc: mxs: Initialize the spinlock prior to using it
From: Stefan Wahren @ 2016-10-28 13:18 UTC (permalink / raw)
To: Fabio Estevam, ulf.hansson; +Cc: kernel, linux-mmc, marex, Fabio Estevam
In-Reply-To: <1477594672-31611-1-git-send-email-festevam@gmail.com>
Hi Fabio,
Am 27.10.2016 um 20:57 schrieb Fabio Estevam:
> From: Fabio Estevam <fabio.estevam@nxp.com>
>
> An interrupt may occur right after devm_request_irq() is called and
> prior to the spinlock initialization, leading to a kernel oops,
> as the interrupt handler uses the spinlock.
>
> In order to prevent this problem, move the spinlock initialization
> prior to requesting the interrupts.
>
> Signed-off-by: Fabio Estevam <fabio.estevam@nxp.com>
this seems to be a bugfix, so please add a fixes tag here.
Maybe this could go to stable.
Thanks
Stefan
^ permalink raw reply
* Re: [PATCH V5 00/25] mmc: mmc: Add Software Command Queuing
From: Adrian Hunter @ 2016-10-28 12:48 UTC (permalink / raw)
To: Ritesh Harjani, Ulf Hansson
Cc: linux-mmc, Alex Lemberg, Mateusz Nowak, Yuliy Izrailov,
Jaehoon Chung, Dong Aisheng, Das Asutosh, Zhangfei Gao,
Dorfman Konstantin, David Griego, Sahitya Tummala,
Venu Byravarasu
In-Reply-To: <76a4ab7d-b4bb-0500-35af-c1a10b51af7c@codeaurora.org>
On 28/10/16 14:48, Ritesh Harjani wrote:
> Hi Adrian,
>
>
> Thanks for the re-base. I am trying to apply this patch series and validate.
> I am seeing some tuning related errors, it could be due to my setup/device.
> I will be working on this.
>
>
> Meanwhile below perf readout was showing some discrepancy (could be that I
> am missing something).
> In your last test both sequential and random read gives almost same
> throughput and even the percentage increase is similar(legacy v/s SW cmdq).
> Could be due to benchmark itself ?
Normally sequential reading would result in large transfers due to
readahead. But the test is using Direct-I/O (-I option) to prevent VFS
caching distorting the results. So the sequential read case ends up being
more like a random read case anyway.
>
> Do you think we should get some other benchmarks tested as well?
> (may be tio?)
>
>
> Also could you please also add some analysis on where we should expect to
> see the score improvement in SW CMDQ and where we may see some decrements
> due to SW CMDQ?
I would expect random reads to be improved, but there is the overhead of the
extra cmdq commands, so a small decrease in performance would be expected
otherwise.
> You did mention in original cover-letter that we mostly should see
> improvement in Random read scores, but below here we are seeing similar or
> higher improvement in sequential reads(4 threads). Which is a bit surprising.
As explained above: the test is called "sequential" but the limited record
size combined with Direct I/O makes it more like random I/O especially when
it is mixed from multiple threads.
>
>
>
>
> On 10/24/2016 2:07 PM, Adrian Hunter wrote:
>> Hi
>>
>> Here is an updated version of the Software Command Queuing patches,
>> re-based on next, with patches 1-5 dropped because they have been applied,
>> and 2 fixes that have been rolled in (refer Changes in V5 below).
>>
>> Performance results:
>>
>> Results can vary from run to run, but here are some results showing 1, 2 or 4
>> processes with 4k and 32k record sizes. They show up to 40% improvement in
>> read performance when there are multiple processes.
>>
>> iozone -s 8192k -r 4k -i 0 -i 1 -i 2 -i 8 -I -t 1 -F /mnt/mmc/iozone1.tmp
>>
>> Children see throughput for 1 initial writers = 27909.87
>> kB/sec 24204.14 kB/sec -13.28 %
>> Children see throughput for 1 rewriters = 28839.28 kB/sec
>> 25531.92 kB/sec -11.47 %
>> Children see throughput for 1 readers = 25889.65
>> kB/sec 24883.23 kB/sec -3.89 %
>> Children see throughput for 1 re-readers = 25558.23 kB/sec
>> 24679.89 kB/sec -3.44 %
>> Children see throughput for 1 random readers = 25571.48
>> kB/sec 24689.52 kB/sec -3.45 %
>> Children see throughput for 1 mixed workload = 25758.59
>> kB/sec 24487.52 kB/sec -4.93 %
>> Children see throughput for 1 random writers = 24787.51
>> kB/sec 19368.99 kB/sec -21.86 %
>>
>> iozone -s 8192k -r 32k -i 0 -i 1 -i 2 -i 8 -I -t 1 -F /mnt/mmc/iozone1.tmp
>>
>> Children see throughput for 1 initial writers = 91344.61
>> kB/sec 102008.56 kB/sec 11.67 %
>> Children see throughput for 1 rewriters = 87932.36 kB/sec
>> 96630.44 kB/sec 9.89 %
>> Children see throughput for 1 readers = 134879.82
>> kB/sec 110292.79 kB/sec -18.23 %
>> Children see throughput for 1 re-readers = 147632.13 kB/sec
>> 109053.33 kB/sec -26.13 %
>> Children see throughput for 1 random readers = 93547.37
>> kB/sec 112225.50 kB/sec 19.97 %
>> Children see throughput for 1 mixed workload = 93560.04
>> kB/sec 110515.21 kB/sec 18.12 %
>> Children see throughput for 1 random writers = 92841.84
>> kB/sec 81153.81 kB/sec -12.59 %
>>
>> iozone -s 8192k -r 4k -i 0 -i 1 -i 2 -i 8 -I -t 2 -F /mnt/mmc/iozone1.tmp
>> /mnt/mmc/iozone2.tmp
>>
>> Children see throughput for 2 initial writers = 31145.43
>> kB/sec 33771.25 kB/sec 8.43 %
>> Children see throughput for 2 rewriters = 30592.57 kB/sec
>> 35916.46 kB/sec 17.40 %
>> Children see throughput for 2 readers = 31669.83
>> kB/sec 37460.13 kB/sec 18.28 %
>> Children see throughput for 2 re-readers = 32079.94 kB/sec
>> 37373.33 kB/sec 16.50 %
>> Children see throughput for 2 random readers = 27731.19
>> kB/sec 37601.65 kB/sec 35.59 %
>> Children see throughput for 2 mixed workload = 13927.50
>> kB/sec 14617.06 kB/sec 4.95 %
>> Children see throughput for 2 random writers = 31250.00
>> kB/sec 33106.72 kB/sec 5.94 %
>>
>> iozone -s 8192k -r 32k -i 0 -i 1 -i 2 -i 8 -I -t 2 -F /mnt/mmc/iozone1.tmp
>> /mnt/mmc/iozone2.tmp
>>
>> Children see throughput for 2 initial writers = 123255.84
>> kB/sec 131252.22 kB/sec 6.49 %
>> Children see throughput for 2 rewriters = 115234.91 kB/sec
>> 107225.74 kB/sec -6.95 %
>> Children see throughput for 2 readers = 128921.86
>> kB/sec 148562.71 kB/sec 15.23 %
>> Children see throughput for 2 re-readers = 127815.24 kB/sec
>> 149304.32 kB/sec 16.81 %
>> Children see throughput for 2 random readers = 125600.46
>> kB/sec 148406.56 kB/sec 18.16 %
>> Children see throughput for 2 mixed workload = 44006.94
>> kB/sec 50937.36 kB/sec 15.75 %
>> Children see throughput for 2 random writers = 120623.95
>> kB/sec 103969.05 kB/sec -13.81 %
>>
>> iozone -s 8192k -r 4k -i 0 -i 1 -i 2 -i 8 -I -t 4 -F /mnt/mmc/iozone1.tmp
>> /mnt/mmc/iozone2.tmp /mnt/mmc/iozone3.tmp /mnt/mmc/iozone4.tmp
>>
>> Children see throughput for 4 initial writers = 24100.96
>> kB/sec 33336.58 kB/sec 38.32 %
>> Children see throughput for 4 rewriters = 31650.20 kB/sec
>> 33091.53 kB/sec 4.55 %
>> Children see throughput for 4 readers = 33276.92
>> kB/sec 41799.89 kB/sec 25.61 %
>> Children see throughput for 4 re-readers = 31786.96 kB/sec
>> 41501.74 kB/sec 30.56 %
>> Children see throughput for 4 random readers = 31991.65
>> kB/sec 40973.93 kB/sec 28.08 %
>> Children see throughput for 4 mixed workload = 15804.80
>> kB/sec 13581.32 kB/sec -14.07 %
>> Children see throughput for 4 random writers = 31231.42
>> kB/sec 34537.03 kB/sec 10.58 %
>>
>> iozone -s 8192k -r 32k -i 0 -i 1 -i 2 -i 8 -I -t 4 -F /mnt/mmc/iozone1.tmp
>> /mnt/mmc/iozone2.tmp /mnt/mmc/iozone3.tmp /mnt/mmc/iozone4.tmp
>>
>> Children see throughput for 4 initial writers = 116567.42
>> kB/sec 119280.35 kB/sec 2.33 %
>> Children see throughput for 4 rewriters = 115010.96 kB/sec
>> 120864.34 kB/sec 5.09 %
>> Children see throughput for 4 readers = 130700.29
>> kB/sec 177834.21 kB/sec 36.06 %
> Do you think sequential read will increase more that of random read. It
> should mostly benefit in random reads right. Any idea why it's behaving
> differently here?
Explained above.
>
>
>> Children see throughput for 4 re-readers = 125392.58 kB/sec
>> 175975.28 kB/sec 40.34 %
>> Children see throughput for 4 random readers = 132194.57
>> kB/sec 176630.46 kB/sec 33.61 %
>> Children see throughput for 4 mixed workload = 56464.98
>> kB/sec 54140.61 kB/sec -4.12 %
>> Children see throughput for 4 random writers = 109128.36
>> kB/sec 85359.80 kB/sec -21.78 %
> Similarly, we don't expect random write scores to decrease here. Do you know
> why this could be the case here?
There is a lot of variation from one run to another
>
>
>>
>>
>> The current block driver supports 2 requests on the go at a time. Patches
>> 1 - 8 make preparations for an arbitrary sized queue. Patches 9 - 12
>> introduce Command Queue definitions and helpers. Patches 13 - 19
>> complete the job of making the block driver use a queue. Patches 20 - 23
>> finally add Software Command Queuing, and 24 - 25 enable it for Intel eMMC
>> controllers. Most of the Software Command Queuing functionality is added
>> in patch 22.
>>
>> As noted below, the patches can also be found here:
>>
>> http://git.infradead.org/users/ahunter/linux-sdhci.git/shortlog/refs/heads/swcmdq
>>
>>
>> Changes in V5:
>>
>> Patches 1-5 dropped because they have been applied.
>>
>> Re-based on next.
>>
>> Fixed use of blk_end_request_cur() when it should have been
>> blk_end_request_all() to error out requests during error recovery.
>>
>> Fixed unpaired retune_hold / retune_release in the error recovery path.
>>
>> Changes in V4:
>>
>> Re-based on next + v4.8-rc2 + "block: Fix secure erase" patch
>>
>> Changes in V3:
>>
>> Patches 1-25 dropped because they have been applied.
>>
>> Re-based on next.
>>
>> mmc: queue: Allocate queue of size qdepth
>> Free queue during cleanup
>>
>> mmc: mmc: Add Command Queue definitions
>> Add cmdq_en to mmc-dev-attrs.txt documentation
>>
>> mmc: queue: Share mmc request array between partitions
>> New patch
>>
>> Changes in V2:
>>
>> Added 5 patches already sent here:
>>
>> http://marc.info/?l=linux-mmc&m=146712062816835
>>
>> Added 3 more new patches:
>>
>> mmc: sdhci-pci: Do not runtime suspend at the end of sdhci_pci_probe()
>> mmc: sdhci: Avoid STOP cmd triggering warning in sdhci_send_command()
>> mmc: sdhci: sdhci_execute_tuning() must delete timer
>>
>> Carried forward the V2 fix to:
>>
>> mmc: mmc_test: Disable Command Queue while mmc_test is used
>>
>> Also reset the cmd circuit for data timeout if it is processing the data
>> cmd, in patch:
>>
>> mmc: sdhci: Do not reset cmd or data circuits that are in use
>>
>> There wasn't much comment on the RFC so there have been few changes.
>> Venu Byravarasu commented that it may be more efficient to use Software
>> Command Queuing only when there is more than 1 request queued - it isn't
>> obvious how well that would work in practice, but it could be added later
>> if it could be shown to be beneficial.
>>
>> Original Cover Letter:
>>
>> Chuanxiao Dong sent some patches last year relating to eMMC 5.1 Software
>> Command Queuing. He did not follow-up but I have contacted him and he says
>> it is OK if I take over upstreaming the patches.
>>
>> eMMC Command Queuing is a feature added in version 5.1. The card maintains
>> a queue of up to 32 data transfers. Commands CMD45/CMD45 are sent to queue
>> up transfers in advance, and then one of the transfers is selected to
>> "execute" by CMD46/CMD47 at which point data transfer actually begins.
>>
>> The advantage of command queuing is that the card can prepare for transfers
>> in advance. That makes a big difference in the case of random reads because
>> the card can start reading into its cache in advance.
>>
>> A v5.1 host controller can manage the command queue itself, but it is also
>> possible for software to manage the queue using an non-v5.1 host controller
>> - that is what Software Command Queuing is.
>>
>> Refer to the JEDEC (http://www.jedec.org/) eMMC v5.1 Specification for more
>> information about Command Queuing.
>>
>> While these patches are heavily based on Dong's patches, there are some
>> changes:
>>
>> SDHCI has been amended to support commands during transfer. That is a
>> generic change added in patches 1 - 5. [Those patches have now been applied]
>> In principle, that would also support SDIO's CMD52 during data transfer.
>>
>> The original approach added multiple commands into the same request for
>> sending CMD44, CMD45 and CMD13. That is not strictly necessary and has
>> been omitted for now.
>>
>> The original approach also called blk_end_request() from the mrq->done()
>> function, which means the upper layers learnt of completed requests
>> slightly earlier. That is not strictly related to Software Command Queuing
>> and is something that could potentially be done for all data requests.
>> That has been omitted for now.
>>
>> The current block driver supports 2 requests on the go at a time. Patches
>> 1 - 8 make preparations for an arbitrary sized queue. Patches 9 - 12
>> introduce Command Queue definitions and helpers. Patches 13 - 19
>> complete the job of making the block driver use a queue. Patches 20 - 23
>> finally add Software Command Queuing, and 24 - 25 enable it for Intel eMMC
>> controllers. Most of the Software Command Queuing functionality is added
>> in patch 22.
>>
>> The patches can also be found here:
>>
>> http://git.infradead.org/users/ahunter/linux-sdhci.git/shortlog/refs/heads/swcmdq
>>
>>
>> The patches have only had basic testing so far. Ad-hoc testing shows a
>> degradation in sequential read performance of about 10% but an increase in
>> throughput for mixed workload of multiple processes of about 90%. The
>> reduction in sequential performance is due to the need to read the Queue
>> Status register between each transfer.
>>
>> These patches should not conflict with Hardware Command Queuing which
>> handles the queue in a completely different way and thus does not need
>> to share code with Software Command Queuing. The exceptions being the
>> Command Queue definitions and queue allocation which should be able to be
>> used.
>>
>>
>> Adrian Hunter (25):
>> mmc: queue: Fix queue thread wake-up
>> mmc: queue: Factor out mmc_queue_alloc_bounce_bufs()
>> mmc: queue: Factor out mmc_queue_alloc_bounce_sgs()
>> mmc: queue: Factor out mmc_queue_alloc_sgs()
>> mmc: queue: Factor out mmc_queue_reqs_free_bufs()
>> mmc: queue: Introduce queue depth
>> mmc: queue: Use queue depth to allocate and free
>> mmc: queue: Allocate queue of size qdepth
>> mmc: mmc: Add Command Queue definitions
>> mmc: mmc: Add functions to enable / disable the Command Queue
>> mmc: mmc_test: Disable Command Queue while mmc_test is used
>> mmc: block: Disable Command Queue while RPMB is used
>> mmc: core: Do not prepare a new request twice
>> mmc: core: Export mmc_retune_hold() and mmc_retune_release()
>> mmc: block: Factor out mmc_blk_requeue()
>> mmc: block: Fix 4K native sector check
>> mmc: block: Use local var for mqrq_cur
>> mmc: block: Pass mqrq to mmc_blk_prep_packed_list()
>> mmc: block: Introduce queue semantics
>> mmc: queue: Share mmc request array between partitions
>> mmc: queue: Add a function to control wake-up on new requests
>> mmc: block: Add Software Command Queuing
>> mmc: mmc: Enable Software Command Queuing
>> mmc: sdhci-pci: Enable Software Command Queuing for some Intel
>> controllers
>> mmc: sdhci-acpi: Enable Software Command Queuing for some Intel
>> controllers
>>
>> Documentation/mmc/mmc-dev-attrs.txt | 1 +
>> drivers/mmc/card/block.c | 738
>> +++++++++++++++++++++++++++++++++---
>> drivers/mmc/card/mmc_test.c | 13 +
>> drivers/mmc/card/queue.c | 332 ++++++++++------
>> drivers/mmc/card/queue.h | 35 +-
>> drivers/mmc/core/core.c | 18 +-
>> drivers/mmc/core/host.c | 2 +
>> drivers/mmc/core/host.h | 2 -
>> drivers/mmc/core/mmc.c | 43 ++-
>> drivers/mmc/core/mmc_ops.c | 27 ++
>> drivers/mmc/host/sdhci-acpi.c | 2 +-
>> drivers/mmc/host/sdhci-pci-core.c | 2 +-
>> include/linux/mmc/card.h | 9 +
>> include/linux/mmc/core.h | 5 +
>> include/linux/mmc/host.h | 4 +-
>> include/linux/mmc/mmc.h | 17 +
>> 16 files changed, 1046 insertions(+), 204 deletions(-)
>>
>>
>> Regards
>> Adrian
>>
>
^ permalink raw reply
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