Add SDHCI Xenon driver

PATCH New Gregory CLEMENT 8 participants 32 messages Jun 9, 2016 – Jun 22, 2016

Subsystems (6)

Device tree bindings MMC, SD and SDIO Tegra architecture support Secure digital host controller interface (SDHCI) driver +2 more subsystems

lore

Share & tools

lore

Gregory CLEMENT

author

2016-06-09 07:10 UTC #0 lore raw

cover Add SDHCI Xenon driver

Hello,

This series adds support for the SDHCI Xenon controller which can be found on the Armada 37xx and the Armada 7K/8K.

This series currently only enables the support on the Armada 37xx SoCs and boards. More tests are needed to enable it on the A7K/A8K.

Following the new trend for the MMC subsystem, instead of introducing new quirks for this controller we extend the mmc/sdhci framework by adding callbacks when needed.

Thanks,

Gregory

Gregory CLEMENT (7):
mmc: sdhci_f_sdh30: restrict voltage_switch() callback to the 1.8V
case
mmc: tegra: restrict the voltage_switch() callback to the 1.8V case
mmc: core: export the alive operation
dt-bindings: mmc: Add Xenon description
arm64: dts: marvell: add eMMC support for Armada 37xx
arm64: dts: marvell: add SDIO support for Armada 37xx
arm64: defconfig: Add Xenon sdhci

Marcin Wojtas (1):
mmc: sdhci: add init_card callback to sdhci

Victor Gu (4):
mmc: sdhci: call voltage_switch callback also for the 3.3V case
mmc: sdhci: add a pre voltage switch callback function
mmc: sdhci: add delay adjust feature including phy reset
mmc: sdhci-xenon: add initial Xenon eMMC driver

 .../devicetree/bindings/mmc/sdhci-xenon.txt        |   27 +
 arch/arm64/boot/dts/marvell/armada-3720-db.dts     |   12 +
 arch/arm64/boot/dts/marvell/armada-37xx.dtsi       |   17 +
 arch/arm64/configs/defconfig                       |    1 +
 drivers/mmc/core/core.c                            |    7 +
 drivers/mmc/host/Kconfig                           |   10 +
 drivers/mmc/host/Makefile                          |    1 +
 drivers/mmc/host/sdhci-tegra.c                     |    4 +
 drivers/mmc/host/sdhci-xenon.c                     | 1164 ++++++++++++++++++++
 drivers/mmc/host/sdhci.c                           |   20 +
 drivers/mmc/host/sdhci.h                           |    3 +
 drivers/mmc/host/sdhci_f_sdh30.c                   |    4 +
 include/linux/mmc/card.h                           |    1 +
 13 files changed, 1271 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mmc/sdhci-xenon.txt
 create mode 100644 drivers/mmc/host/sdhci-xenon.c

Signature · 1 line

2.5.0

Gregory CLEMENT

author

2016-06-09 07:10 UTC #1 lore raw

PATCH 1/12 mmc: sdhci: add init_card callback to sdhci

From: Marcin Wojtas <[email protected]>

Some sdhci hosts may require handling quirks during card initialization at the time when its type is already known. Hence a new callback (init_card) is added in sdhci_ops.

Signed-off-by Marcin Wojtas <[email protected]>

Signed-off-by Gregory CLEMENT <[email protected]>

---
 drivers/mmc/host/sdhci.c | 9 +++++++++
 drivers/mmc/host/sdhci.h | 1 +
 2 files changed, 10 insertions(+)

drivers/mmc/host/sdhci.c +9 -0

index 0e3d7c056cb1..fd4e9a84f739 100644
@@ -2117,6 +2117,14 @@ static void sdhci_card_event(struct mmc_host *mmc)
 	spin_unlock_irqrestore(&host->lock, flags);
 }
 
+static void sdhci_init_card(struct mmc_host *mmc, struct mmc_card *card)
+{
+	struct sdhci_host *host = mmc_priv(mmc);
+
+	if (host->ops->init_card)
+		host->ops->init_card(host, card);
+}
+
 static const struct mmc_host_ops sdhci_ops = {
 	.request	= sdhci_request,
 	.post_req	= sdhci_post_req,

@@ -2132,6 +2140,7 @@ static const struct mmc_host_ops sdhci_ops = {
 	.select_drive_strength		= sdhci_select_drive_strength,
 	.card_event			= sdhci_card_event,
 	.card_busy	= sdhci_card_busy,
+	.init_card	= sdhci_init_card,
 };
 
 /*****************************************************************************\

drivers/mmc/host/sdhci.h +1 -0

index 609f87ca536b..49c6c5b0e33b 100644
@@ -549,6 +549,7 @@ struct sdhci_ops {
 					 struct mmc_card *card,
 					 unsigned int max_dtr, int host_drv,
 					 int card_drv, int *drv_type);
+	void	(*init_card)(struct sdhci_host *host, struct mmc_card *card);
 };
 
 #ifdef CONFIG_MMC_SDHCI_IO_ACCESSORS

Signature · 1 line

2.5.0

Adrian Hunter

maintainer

2016-06-13 08:32 UTC #2 lore raw

PATCH 1/12 mmc: sdhci: add init_card callback to sdhci

Quoted 5 lines · Gregory CLEMENT

From: Marcin Wojtas <[email protected]>
Some sdhci hosts may require handling quirks during card initialization at the time when its type is already known. Hence a new callback (init_card) is added in sdhci_ops.

I plan to remove init_card and other host ops that sdhci doesn't use (like sdhci_select_drive_strength). The driver must provide the mmc op instead. e.g. in sdhci-xenon.c

	host->mmc_host_ops.init_card = sdhci_xenon_init_card

Quoted 48 lines

Signed-off-by Marcin Wojtas <[email protected]>

Signed-off-by Gregory CLEMENT <[email protected]>

---
 drivers/mmc/host/sdhci.c | 9 +++++++++
 drivers/mmc/host/sdhci.h | 1 +
 2 files changed, 10 insertions(+)

drivers/mmc/host/sdhci.c +9 -0

index 0e3d7c056cb1..fd4e9a84f739 100644
@@ -2117,6 +2117,14 @@ static void sdhci_card_event(struct mmc_host *mmc)
 	spin_unlock_irqrestore(&host->lock, flags);
 }
 
+static void sdhci_init_card(struct mmc_host *mmc, struct mmc_card *card)
+{
+	struct sdhci_host *host = mmc_priv(mmc);
+
+	if (host->ops->init_card)
+		host->ops->init_card(host, card);
+}
+
 static const struct mmc_host_ops sdhci_ops = {
 	.request	= sdhci_request,
 	.post_req	= sdhci_post_req,

@@ -2132,6 +2140,7 @@ static const struct mmc_host_ops sdhci_ops = {
 	.select_drive_strength		= sdhci_select_drive_strength,
 	.card_event			= sdhci_card_event,
 	.card_busy	= sdhci_card_busy,
+	.init_card	= sdhci_init_card,
 };
 
 /*****************************************************************************\

drivers/mmc/host/sdhci.h +1 -0

index 609f87ca536b..49c6c5b0e33b 100644
@@ -549,6 +549,7 @@ struct sdhci_ops {
 					 struct mmc_card *card,
 					 unsigned int max_dtr, int host_drv,
 					 int card_drv, int *drv_type);
+	void	(*init_card)(struct sdhci_host *host, struct mmc_card *card);
 };
 
 #ifdef CONFIG_MMC_SDHCI_IO_ACCESSORS

Gregory CLEMENT

author

2016-06-14 07:58 UTC #3 lore raw

PATCH 1/12 mmc: sdhci: add init_card callback to sdhci

Hi Adrian,

Quoted 7 lines · Adrian Hunter

Quoted 5 lines · Gregory CLEMENT
From: Marcin Wojtas <[email protected]>
Some sdhci hosts may require handling quirks during card initialization at the time when its type is already known. Hence a new callback (init_card) is added in sdhci_ops.

Quoted 5 lines

I plan to remove init_card and other host ops that sdhci doesn't use (like sdhci_select_drive_strength). The driver must provide the mmc op instead. e.g. in sdhci-xenon.c
	host->mmc_host_ops.init_card = sdhci_xenon_init_card

OK I will do this.

Thanks for your review.

Gregory

Quoted 55 lines

Quoted 42 lines

Signed-off-by Marcin Wojtas <[email protected]>

Signed-off-by Gregory CLEMENT <[email protected]>

---
 drivers/mmc/host/sdhci.c | 9 +++++++++
 drivers/mmc/host/sdhci.h | 1 +
 2 files changed, 10 insertions(+)

drivers/mmc/host/sdhci.c +9 -0

index 0e3d7c056cb1..fd4e9a84f739 100644
@@ -2117,6 +2117,14 @@ static void sdhci_card_event(struct mmc_host *mmc)
 	spin_unlock_irqrestore(&host->lock, flags);
 }
 
+static void sdhci_init_card(struct mmc_host *mmc, struct mmc_card *card)
+{
+	struct sdhci_host *host = mmc_priv(mmc);
+
+	if (host->ops->init_card)
+		host->ops->init_card(host, card);
+}
+
 static const struct mmc_host_ops sdhci_ops = {
 	.request	= sdhci_request,
 	.post_req	= sdhci_post_req,

@@ -2132,6 +2140,7 @@ static const struct mmc_host_ops sdhci_ops = {
 	.select_drive_strength		= sdhci_select_drive_strength,
 	.card_event			= sdhci_card_event,
 	.card_busy	= sdhci_card_busy,
+	.init_card	= sdhci_init_card,
 };
 
 /*****************************************************************************\

drivers/mmc/host/sdhci.h +0 -0

index 609f87ca536b..49c6c5b0e33b 100644

drivers/mmc/host/sdhci.h struct sdhci_ops { · line 555

Quoted 7 lines

drivers/mmc/host/sdhci.h +1 -0

@@ -549,6 +549,7 @@ struct sdhci_ops {
 					 struct mmc_card *card,
 					 unsigned int max_dtr, int host_drv,
 					 int card_drv, int *drv_type);
+	void	(*init_card)(struct sdhci_host *host, struct mmc_card *card);
 };
 
 #ifdef CONFIG_MMC_SDHCI_IO_ACCESSORS

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Signature · 4 lines

Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

Gregory CLEMENT

author

2016-06-09 07:10 UTC #4 lore raw

PATCH 2/12 mmc: sdhci_f_sdh30: restrict voltage_switch() callback to the 1.8V case

In reply to Gregory CLEMENT

Given its name the voltage_switch() should be called for any voltage. However currently it is only called for the 1.8V case.

That means that the current implementation of the callback could have made this assumption. It seems the case for the f_sdh3 driver. In order to not adding a regression when the voltage_switch() function will be called for the other voltage targets, this patch ensure that the code of the current callback won't be executed for the other case than 1.8V.

Cc Vincent Yang <[email protected]>

Signed-off-by Gregory CLEMENT <[email protected]>

---
 drivers/mmc/host/sdhci_f_sdh30.c | 4 ++++
 1 file changed, 4 insertions(+)

drivers/mmc/host/sdhci_f_sdh30.c +4 -0

index 983b8b32ef96..09a341cbd572 100644
@@ -54,6 +54,10 @@ static void sdhci_f_sdh30_soft_voltage_switch(struct sdhci_host *host)
 	struct f_sdhost_priv *priv = sdhci_priv(host);
 	u32 ctrl = 0;
 
+	/* Initially this function was called only for the 1.8V case */
+	if (host->mmc->ios.signal_voltage != MMC_SIGNAL_VOLTAGE_180)
+		return;
+
 	usleep_range(2500, 3000);
 	ctrl = sdhci_readl(host, F_SDH30_IO_CONTROL2);
 	ctrl |= F_SDH30_CRES_O_DN;

Signature · 1 line

2.5.0

Gregory CLEMENT

author

2016-06-09 07:10 UTC #5 lore raw

PATCH 3/12 mmc: tegra: restrict the voltage_switch() callback to the 1.8V case

In reply to Gregory CLEMENT

Given its name the voltage_switch() should be called for any voltage. However currently it is only called for the 1.8V case.

That means that the current implementation of the callback could have made this assumption. In order to not adding a regression when the voltage_switch() function will be called for the other voltage targets, this patch ensure that the code of the current callback won't be executed for the other case than 1.8V.

Cc Lucas Stach <[email protected]>

Signed-off-by Gregory CLEMENT <[email protected]>

---
 drivers/mmc/host/sdhci-tegra.c | 4 ++++
 1 file changed, 4 insertions(+)

drivers/mmc/host/sdhci-tegra.c +4 -0

index bcc0de47fe7e..01f58296125e 100644
@@ -302,6 +302,10 @@ static void tegra_sdhci_voltage_switch(struct sdhci_host *host)
 	struct sdhci_tegra *tegra_host = sdhci_pltfm_priv(pltfm_host);
 	const struct sdhci_tegra_soc_data *soc_data = tegra_host->soc_data;
 
+	/* initially this function was called only for the 1.8V case */
+	if (host->mmc->ios.signal_voltage != MMC_SIGNAL_VOLTAGE_180)
+		return;
+
 	if (soc_data->nvquirks & NVQUIRK_HAS_PADCALIB)
 		tegra_host->pad_calib_required = true;
 }

Signature · 1 line

2.5.0

Lucas Stach

2016-06-09 09:54 UTC #6 lore raw

PATCH 3/12 mmc: tegra: restrict the voltage_switch() callback to the 1.8V case

Am Donnerstag, den 09.06.2016, 09:10 +0200 schrieb Gregory CLEMENT:

Quoted 11 lines

Given its name the voltage_switch() should be called for any voltage. However currently it is only called for the 1.8V case.
That means that the current implementation of the callback could have made this assumption. In order to not adding a regression when the voltage_switch() function will be called for the other voltage targets,
this patch ensure that the code of the current callback won't be executed
for the other case than 1.8V.

This isn't necessary, it's fine to re-execute the pad calibration for any voltage switch.

Regards,
Lucas

Quoted 26 lines

Cc Lucas Stach <[email protected]>
Signed-off-by Gregory CLEMENT <[email protected]>
---
?drivers/mmc/host/sdhci-tegra.c | 4 ++++
?1 file changed, 4 insertions(+)
drivers/mmc/host/sdhci- renamed from drivers/mmc/host/sdhci-tegra.c +0 -0
tegra.c
index bcc0de47fe7e..01f58296125e 100644
drivers/mmc/host/sdhci-tegra.c +0 -0
@@ -302,6 +302,10 @@ static void tegra_sdhci_voltage_switch(struct
 sdhci_host *host)
? struct sdhci_tegra *tegra_host =
sdhci_pltfm_priv(pltfm_host);
? const struct sdhci_tegra_soc_data *soc_data = tegra_host-
Quoted 1 line
soc_data;
?
drivers/mmc/host/sdhci-tegra.c +1 -0
+	/* initially this function was called only for the 1.8V case
*/
drivers/mmc/host/sdhci-tegra.c +3 -0
+	if (host->mmc->ios.signal_voltage != MMC_SIGNAL_VOLTAGE_180)
+		return;
+
? if (soc_data->nvquirks & NVQUIRK_HAS_PADCALIB)
? tegra_host->pad_calib_required = true;
?}

Gregory CLEMENT

author

2016-06-09 12:27 UTC #7 lore raw

PATCH 3/12 mmc: tegra: restrict the voltage_switch() callback to the 1.8V case

Hi Lucas,

Quoted 12 lines · Lucas Stach

Am Donnerstag, den 09.06.2016, 09:10 +0200 schrieb Gregory CLEMENT:
Quoted 11 lines
Given its name the voltage_switch() should be called for any voltage. However currently it is only called for the 1.8V case.
That means that the current implementation of the callback could have made this assumption. In order to not adding a regression when the voltage_switch() function will be called for the other voltage targets,
this patch ensure that the code of the current callback won't be executed
for the other case than 1.8V.

Quoted 2 lines

This isn't necessary, it's fine to re-execute the pad calibration for any voltage switch.

Thanks for your feedback, so I can remove this patch from the series.

Gregory

Quoted 30 lines

Regards,
Lucas
Quoted 26 lines
Cc Lucas Stach <[email protected]>
Signed-off-by Gregory CLEMENT <[email protected]>
---
?drivers/mmc/host/sdhci-tegra.c | 4 ++++
?1 file changed, 4 insertions(+)
drivers/mmc/host/sdhci- renamed from drivers/mmc/host/sdhci-tegra.c +0 -0
tegra.c
index bcc0de47fe7e..01f58296125e 100644
drivers/mmc/host/sdhci-tegra.c +0 -0
@@ -302,6 +302,10 @@ static void tegra_sdhci_voltage_switch(struct
 sdhci_host *host)
? struct sdhci_tegra *tegra_host =
sdhci_pltfm_priv(pltfm_host);
? const struct sdhci_tegra_soc_data *soc_data = tegra_host-
Quoted 1 line
soc_data;
?
drivers/mmc/host/sdhci-tegra.c +1 -0
+	/* initially this function was called only for the 1.8V case
*/
drivers/mmc/host/sdhci-tegra.c +3 -0
+	if (host->mmc->ios.signal_voltage != MMC_SIGNAL_VOLTAGE_180)
+		return;
+
? if (soc_data->nvquirks & NVQUIRK_HAS_PADCALIB)
? tegra_host->pad_calib_required = true;
?}

Signature · 4 lines

Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

Gregory CLEMENT

author

2016-06-09 07:10 UTC #8 lore raw

PATCH 4/12 mmc: sdhci: call voltage_switch callback also for the 3.3V case

In reply to Gregory CLEMENT

From: Victor Gu <[email protected]>

Currently the voltage switch callback function is only called in the 1.8V case. However, given the name this callback should be called in the 3.3V case too. It is needed for the Xenon controller.

[gregory.clement at free-electrons.com: split the initial commit and modify the log]

Signed-off-by Victor Gu <[email protected]>

Signed-off-by Marcin Wojtas <[email protected]>

Signed-off-by Gregory CLEMENT <[email protected]>

---
 drivers/mmc/host/sdhci.c | 4 ++++
 1 file changed, 4 insertions(+)

drivers/mmc/host/sdhci.c +4 -0

index fd4e9a84f739..b07219426d39 100644
@@ -1749,6 +1749,10 @@ static int sdhci_start_signal_voltage_switch(struct mmc_host *mmc,
 		/* Wait for 5ms */
 		usleep_range(5000, 5500);
 
+		/* Some controller need to do more when switching */
+		if (host->ops->voltage_switch)
+			host->ops->voltage_switch(host);
+
 		/* 3.3V regulator output should be stable within 5 ms */
 		ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);
 		if (!(ctrl & SDHCI_CTRL_VDD_180))

Signature · 1 line

2.5.0

Gregory CLEMENT

author

2016-06-09 07:10 UTC #9 lore raw

PATCH 5/12 mmc: sdhci: add a pre voltage switch callback function

In reply to Gregory CLEMENT

From: Victor Gu <[email protected]>

Some host controller such as Xenon needs additional setting when switching signal voltage in eMMC mode. They also need to re-enable internal clock before a voltage switch.

This commit adds a callback routine "voltage_switch_pre" in the struct sdhci_ops, which is used by some host controllers which need re-enable the internal clock before a voltage switch.

[gregory.clement at free-electrons.com: split the initial commit and reformulate the log]

Signed-off-by Victor Gu <[email protected]>

Signed-off-by Marcin Wojtas <[email protected]>

Signed-off-by Gregory CLEMENT <[email protected]>

---
 drivers/mmc/host/sdhci.c | 4 ++++
 drivers/mmc/host/sdhci.h | 1 +
 2 files changed, 5 insertions(+)

drivers/mmc/host/sdhci.c +4 -0

index b07219426d39..cad03ffa9d9b 100644
@@ -1729,6 +1729,10 @@ static int sdhci_start_signal_voltage_switch(struct mmc_host *mmc,
 	if (host->version < SDHCI_SPEC_300)
 		return 0;
 
+	/* Some controller need to do more before switching */
+	if (host->ops->voltage_switch_pre)
+		host->ops->voltage_switch_pre(host);
+
 	ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);
 
 	switch (ios->signal_voltage) {

drivers/mmc/host/sdhci.h +1 -0

index 49c6c5b0e33b..6bec1b0368d2 100644
@@ -550,6 +550,7 @@ struct sdhci_ops {
 					 unsigned int max_dtr, int host_drv,
 					 int card_drv, int *drv_type);
 	void	(*init_card)(struct sdhci_host *host, struct mmc_card *card);
+	void	(*voltage_switch_pre)(struct sdhci_host *host);
 };
 
 #ifdef CONFIG_MMC_SDHCI_IO_ACCESSORS

Signature · 1 line

2.5.0

Adrian Hunter

maintainer

2016-06-13 08:46 UTC #10 lore raw

PATCH 5/12 mmc: sdhci: add a pre voltage switch callback function

Quoted 9 lines · Gregory CLEMENT

From: Victor Gu <[email protected]>
Some host controller such as Xenon needs additional setting when switching signal voltage in eMMC mode. They also need to re-enable internal clock before a voltage switch.
This commit adds a callback routine "voltage_switch_pre" in the struct sdhci_ops, which is used by some host controllers which need re-enable the internal clock before a voltage switch.

Don't want to add sdhci host ops for the "do something before a mmc host op" case.

Instead, export sdhci_start_signal_voltage_switch() and hook host->mmc_host_ops.start_signal_voltage_switch. Then in sdhci-xenon.c:

int xenon__start_signal_voltage_switch(struct mmc_host *mmc,
struct mmc_ios *ios)

{
	blah blah
	return sdhci_start_signal_voltage_switch(mmc, ios);
}

Quoted 40 lines

[gregory.clement at free-electrons.com: split the initial commit and reformulate the log]

Signed-off-by Victor Gu <[email protected]>

Signed-off-by Marcin Wojtas <[email protected]>

Signed-off-by Gregory CLEMENT <[email protected]>

---
 drivers/mmc/host/sdhci.c | 4 ++++
 drivers/mmc/host/sdhci.h | 1 +
 2 files changed, 5 insertions(+)

drivers/mmc/host/sdhci.c +4 -0

index b07219426d39..cad03ffa9d9b 100644
@@ -1729,6 +1729,10 @@ static int sdhci_start_signal_voltage_switch(struct mmc_host *mmc,
 	if (host->version < SDHCI_SPEC_300)
 		return 0;
 
+	/* Some controller need to do more before switching */
+	if (host->ops->voltage_switch_pre)
+		host->ops->voltage_switch_pre(host);
+
 	ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);
 
 	switch (ios->signal_voltage) {

drivers/mmc/host/sdhci.h +1 -0

index 49c6c5b0e33b..6bec1b0368d2 100644
@@ -550,6 +550,7 @@ struct sdhci_ops {
 					 unsigned int max_dtr, int host_drv,
 					 int card_drv, int *drv_type);
 	void	(*init_card)(struct sdhci_host *host, struct mmc_card *card);
+	void	(*voltage_switch_pre)(struct sdhci_host *host);
 };
 
 #ifdef CONFIG_MMC_SDHCI_IO_ACCESSORS

Ziji Hu

maintainer

2016-06-13 14:04 UTC #11 lore raw

PATCH 5/12 mmc: sdhci: add a pre voltage switch callback function

Hi Adrian,

Thanks a lot for the suggestion. I'm from Marvell Xenon SDHC team.
Most of sdhci host ops are still used in our driver. We just change or add few ops. Thus, if we create our own mmc_host_ops, many static sdhci host ops will be exported. I would like to know whether such a modification can be accepted or not.

I notice that community is improving sdhci. Thus all the static sdhci ops called outside the sdhci.c will be exported. Is my understanding correct? Thank you.

Best regards,
Hu Ziji

Quoted 71 lines

Quoted 5 lines
? 2016?6?13??16:52?Adrian Hunter <[email protected]> ???
On 09/06/16 10:10, Gregory CLEMENT wrote:
From: Victor Gu <[email protected]>
Quoted 7 lines
Some host controller such as Xenon needs additional setting when switching signal voltage in eMMC mode. They also need to re-enable internal clock before a voltage switch.
This commit adds a callback routine "voltage_switch_pre" in the struct sdhci_ops, which is used by some host controllers which need re-enable the internal clock before a voltage switch.
Don't want to add sdhci host ops for the "do something before a mmc host op" case.
Instead, export sdhci_start_signal_voltage_switch() and hook host->mmc_host_ops.start_signal_voltage_switch. Then in sdhci-xenon.c:
int xenon__start_signal_voltage_switch(struct mmc_host *mmc,
struct mmc_ios *ios)
{
blah blah
return sdhci_start_signal_voltage_switch(mmc, ios);
}
Quoted 32 lines
[gregory.clement at free-electrons.com: split the initial commit and reformulate the log]
Signed-off-by Victor Gu <[email protected]>
Signed-off-by Marcin Wojtas <[email protected]>
Signed-off-by Gregory CLEMENT <[email protected]>
---
drivers/mmc/host/sdhci.c | 4 ++++
drivers/mmc/host/sdhci.h | 1 +
2 files changed, 5 insertions(+)
drivers/mmc/host/sdhci.c +4 -0
index b07219426d39..cad03ffa9d9b 100644
@@ -1729,6 +1729,10 @@ static int sdhci_start_signal_voltage_switch(struct mmc_host *mmc,
  if (host->version < SDHCI_SPEC_300)
      return 0;
 
+    /* Some controller need to do more before switching */
+    if (host->ops->voltage_switch_pre)
+        host->ops->voltage_switch_pre(host);
+
  ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);
 
  switch (ios->signal_voltage) {
drivers/mmc/host/sdhci.h +0 -0
index 49c6c5b0e33b..6bec1b0368d2 100644
drivers/mmc/host/sdhci.h struct sdhci_ops { · line 556
Quoted 7 lines
drivers/mmc/host/sdhci.h +1 -0
@@ -550,6 +550,7 @@ struct sdhci_ops {
                   unsigned int max_dtr, int host_drv,
                   int card_drv, int *drv_type);
  void    (*init_card)(struct sdhci_host *host, struct mmc_card *card);
+    void    (*voltage_switch_pre)(struct sdhci_host *host);
 };
 
 #ifdef CONFIG_MMC_SDHCI_IO_ACCESSORS
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Adrian Hunter

maintainer

2016-06-14 07:07 UTC #12 lore raw

PATCH 5/12 mmc: sdhci: add a pre voltage switch callback function

Quoted 11 lines · Ziji Hu

Hi Adrian,
Thanks a lot for the suggestion. I'm from Marvell Xenon SDHC team. Most of sdhci host ops are still used in our driver. We just change or add few ops. Thus, if we create our own mmc_host_ops, many static sdhci host ops will be exported. I would like to know whether such a modification can be accepted or not.
I notice that community is improving sdhci. Thus all the static sdhci ops called outside the sdhci.c will be exported. Is my understanding correct? Thank you.

The aim is to make sdhci more like a library and give more control to the drivers. That will mean more sdhci functions get exported, particularly mmc host ops like sdhci_set_ios() or sdhci_start_signal_voltage_switch().

Quoted 60 lines

Best regards, Hu Ziji
Quoted 57 lines
Quoted 5 lines
? 2016?6?13??16:52?Adrian Hunter <[email protected]> ???
On 09/06/16 10:10, Gregory CLEMENT wrote: From: Victor Gu <[email protected]>
Quoted 7 lines
Some host controller such as Xenon needs additional setting when switching signal voltage in eMMC mode. They also need to re-enable internal clock before a voltage switch.
This commit adds a callback routine "voltage_switch_pre" in the struct sdhci_ops, which is used by some host controllers which need re-enable the internal clock before a voltage switch.
Don't want to add sdhci host ops for the "do something before a mmc host op" case.
Instead, export sdhci_start_signal_voltage_switch() and hook host->mmc_host_ops.start_signal_voltage_switch. Then in sdhci-xenon.c:
int xenon__start_signal_voltage_switch(struct mmc_host *mmc, struct mmc_ios *ios) { blah blah return sdhci_start_signal_voltage_switch(mmc, ios); }
Quoted 28 lines
[gregory.clement at free-electrons.com: split the initial commit and reformulate the log]
Signed-off-by Victor Gu <[email protected]> Signed-off-by: Marcin
Wojtas <[email protected]> Signed-off-by: Gregory CLEMENT <[email protected]> --- drivers/mmc/host/sdhci.c | 4 ++++ drivers/mmc/host/sdhci.h | 1 + 2 files changed, 5 insertions(+)
drivers/mmc/host/sdhci.c +0 -0
index b07219426d39..cad03ffa9d9b 100644 ---
a/drivers/mmc/host/sdhci.c +++ b/drivers/mmc/host/sdhci.c @@ -1729,6 +1729,10 @@ static int sdhci_start_signal_voltage_switch(struct mmc_host *mmc, if (host->version < SDHCI_SPEC_300) return 0;
+ /* Some controller need to do more before switching */ + if
(host->ops->voltage_switch_pre) + host->ops->voltage_switch_pre(host); + ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);
switch (ios->signal_voltage) { diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h index 49c6c5b0e33b..6bec1b0368d2 100644 --- a/drivers/mmc/host/sdhci.h +++ b/drivers/mmc/host/sdhci.h @@ -550,6 +550,7 @@ struct sdhci_ops { unsigned int max_dtr, int
host_drv, int card_drv, int *drv_type); void (*init_card)(struct
sdhci_host *host, struct mmc_card *card); + void
(*voltage_switch_pre)(struct sdhci_host *host); };
Quoted 1 line
#ifdef CONFIG_MMC_SDHCI_IO_ACCESSORS
-- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo at vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html

Gregory CLEMENT

author

2016-06-14 07:59 UTC #13 lore raw

PATCH 5/12 mmc: sdhci: add a pre voltage switch callback function

In reply to Adrian Hunter

Hi Adrian,

Quoted 17 lines · Adrian Hunter

Quoted 9 lines · Gregory CLEMENT
From: Victor Gu <[email protected]>
Some host controller such as Xenon needs additional setting when switching signal voltage in eMMC mode. They also need to re-enable internal clock before a voltage switch.
This commit adds a callback routine "voltage_switch_pre" in the struct sdhci_ops, which is used by some host controllers which need re-enable the internal clock before a voltage switch.
Don't want to add sdhci host ops for the "do something before a mmc host op" case.
Instead, export sdhci_start_signal_voltage_switch() and hook host->mmc_host_ops.start_signal_voltage_switch. Then in sdhci-xenon.c:

Quoted 6 lines

int xenon__start_signal_voltage_switch(struct mmc_host *mmc,
struct mmc_ios *ios)
{
	blah blah
	return sdhci_start_signal_voltage_switch(mmc, ios);
}

OK I see your point.

Thanks,

Gregory

Quoted 43 lines

Quoted 42 lines

[gregory.clement at free-electrons.com: split the initial commit and reformulate the log]

Signed-off-by Victor Gu <[email protected]>

Signed-off-by Marcin Wojtas <[email protected]>

Signed-off-by Gregory CLEMENT <[email protected]>

---
 drivers/mmc/host/sdhci.c | 4 ++++
 drivers/mmc/host/sdhci.h | 1 +
 2 files changed, 5 insertions(+)

drivers/mmc/host/sdhci.c +4 -0

index b07219426d39..cad03ffa9d9b 100644
@@ -1729,6 +1729,10 @@ static int sdhci_start_signal_voltage_switch(struct mmc_host *mmc,
 	if (host->version < SDHCI_SPEC_300)
 		return 0;
 
+	/* Some controller need to do more before switching */
+	if (host->ops->voltage_switch_pre)
+		host->ops->voltage_switch_pre(host);
+
 	ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);
 
 	switch (ios->signal_voltage) {

drivers/mmc/host/sdhci.h +1 -0

index 49c6c5b0e33b..6bec1b0368d2 100644
@@ -550,6 +550,7 @@ struct sdhci_ops {
 					 unsigned int max_dtr, int host_drv,
 					 int card_drv, int *drv_type);
 	void	(*init_card)(struct sdhci_host *host, struct mmc_card *card);
+	void	(*voltage_switch_pre)(struct sdhci_host *host);
 };
 
 #ifdef CONFIG_MMC_SDHCI_IO_ACCESSORS

Signature · 4 lines

Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

Gregory CLEMENT

author

2016-06-09 07:10 UTC #14 lore raw

PATCH 6/12 mmc: sdhci: add delay adjust feature including phy reset

In reply to Gregory CLEMENT

From: Victor Gu <[email protected]>

Tuning is only available in HS200 mode. However, some host controllers, such as the Xenon one, need to adjust delay for other modes and even adjust the delay before tuning.

This commit adds a callback routine "delay_adj" in the struct sdhci_ops.

[gregory.clement at free-electrons.com: split the initial commit and reformulate the log]

Signed-off-by Victor Gu <[email protected]>

Signed-off-by Gregory CLEMENT <[email protected]>

---
 drivers/mmc/host/sdhci.c | 3 +++
 drivers/mmc/host/sdhci.h | 1 +
 2 files changed, 4 insertions(+)

drivers/mmc/host/sdhci.c +3 -0

index cad03ffa9d9b..2acae2b3b2d3 100644
@@ -1606,6 +1606,9 @@ static void sdhci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 
 	mmiowb();
 	spin_unlock_irqrestore(&host->lock, flags);
+
+	if (host->ops->delay_adj)
+		host->ops->delay_adj(host, ios);
 }
 
 static int sdhci_get_cd(struct mmc_host *mmc)

drivers/mmc/host/sdhci.h +1 -0

index 6bec1b0368d2..f88f72782a8d 100644
@@ -551,6 +551,7 @@ struct sdhci_ops {
 					 int card_drv, int *drv_type);
 	void	(*init_card)(struct sdhci_host *host, struct mmc_card *card);
 	void	(*voltage_switch_pre)(struct sdhci_host *host);
+	int	(*delay_adj)(struct sdhci_host *host, struct mmc_ios *ios);
 };
 
 #ifdef CONFIG_MMC_SDHCI_IO_ACCESSORS

Signature · 1 line

2.5.0

Gregory CLEMENT

author

2016-06-09 07:10 UTC #15 lore raw

PATCH 7/12 mmc: core: export the alive operation

In reply to Gregory CLEMENT

Depending of the type of card there is a different operation to know if the car is alive. However, the access to this operation are private. This patch introduce a new function allowing to know if the card is alive by using the appropriate operation.

Signed-off-by Gregory CLEMENT <[email protected]>

---
 drivers/mmc/core/core.c  | 7 +++++++
 include/linux/mmc/card.h | 1 +
 2 files changed, 8 insertions(+)

drivers/mmc/core/core.c +7 -0

index 8b4dfd45433b..3dce095c5f89 100644
@@ -2464,6 +2464,13 @@ int mmc_hw_reset(struct mmc_host *host)
 }
 EXPORT_SYMBOL(mmc_hw_reset);
 
+
+int card_alive(struct mmc_card *card)
+{
+	return card->host->bus_ops->alive(card->host);
+}
+EXPORT_SYMBOL(card_alive);
+
 static int mmc_rescan_try_freq(struct mmc_host *host, unsigned freq)
 {
 	host->f_init = freq;

include/linux/mmc/card.h +1 -0

index eb0151bac50c..287a0b12d8ba 100644
@@ -526,6 +526,7 @@ struct mmc_driver {
 	void (*shutdown)(struct mmc_card *);
 };
 
+extern int card_alive(struct mmc_card *);
 extern int mmc_register_driver(struct mmc_driver *);
 extern void mmc_unregister_driver(struct mmc_driver *);

Signature · 1 line

2.5.0

Gregory CLEMENT

author

2016-06-09 07:10 UTC #16 lore raw

PATCH 8/12 dt-bindings: mmc: Add Xenon description

In reply to Gregory CLEMENT

This patch documents the Xenon SDHCI controller that we can find in the Armada 37xx and the Armada 7K and 8K SoCs.

Signed-off-by Gregory CLEMENT <[email protected]>

---
 .../devicetree/bindings/mmc/sdhci-xenon.txt        | 27 ++++++++++++++++++++++
 1 file changed, 27 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mmc/sdhci-xenon.txt

Documentation/devicetree/bindings/mmc/sdhci-xenon.txt new file +27 -0

new file mode 100644
index 000000000000..e8d49fda5f12
@@ -0,0 +1,27 @@
+* Marvell sdhci-xenon controller
+
+This file documents differences between the core properties in mmc.txt
+and the properties used by the sdhci-xenon drivers.
+
+Required properties:
+- compatible: Should be "marvell,xenon-sdhci".
+- reg:
+  one register area for the SDHCI registers.
+- interrupts-extended: interrupt number and attributes
+- clocks: array of clocks required for SDHCI.
+- bus-width: max bus width in unit of bit, normally should be 1 bit, 4 bits or 8 bits
+
+Optional properties:
+- non-removable: whether card is non-removable.
+- auto-cmd12: whether cmd12 is sent automatically.
+Example:
+
+sdhci0: sdhci at D8000 {
+	compatible = "marvell,xenon-sdhci";
+	reg = <0xD8000 0x300>;
+	interrupts-extended = <&gic GIC_SPI 26 IRQ_TYPE_LEVEL_HIGH>;
+	non-removable;
+	bus-width = <8>;
+	auto-cmd12;
+	status = "disabled";
+};

Signature · 1 line

2.5.0

Rob Herring

maintainer

2016-06-10 14:16 UTC #17 lore raw

PATCH 8/12 dt-bindings: mmc: Add Xenon description

Quoted 15 lines · Gregory CLEMENT

This patch documents the Xenon SDHCI controller that we can find in the Armada 37xx and the Armada 7K and 8K SoCs.
Signed-off-by Gregory CLEMENT <[email protected]>
---
 .../devicetree/bindings/mmc/sdhci-xenon.txt        | 27 ++++++++++++++++++++++
 1 file changed, 27 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mmc/sdhci-xenon.txt
Documentation/devicetree/bindings/mmc/sdhci-xenon.txt new file +0 -0
new file mode 100644
index 000000000000..e8d49fda5f12

Documentation/devicetree/bindings/mmc/sdhci-xenon.txt line 4

Quoted 4 lines

Documentation/devicetree/bindings/mmc/sdhci-xenon.txt new file +4 -0

@@ -0,0 +1,27 @@
+* Marvell sdhci-xenon controller
+
+This file documents differences between the core properties in mmc.txt
+and the properties used by the sdhci-xenon drivers.

Bindings don't document drivers...

Documentation/devicetree/bindings/mmc/sdhci-xenon.txt line 7

Quoted 3 lines

Documentation/devicetree/bindings/mmc/sdhci-xenon.txt new file +3 -0
+
+Required properties:
+- compatible: Should be "marvell,xenon-sdhci".

SoC specific compatible strings please. This one can be fallback.

Documentation/devicetree/bindings/mmc/sdhci-xenon.txt line 10

Quoted 3 lines

Documentation/devicetree/bindings/mmc/sdhci-xenon.txt new file +3 -0
+- reg:
+  one register area for the SDHCI registers.
+- interrupts-extended: interrupt number and attributes

Just use interrupts unless you really have a need for interrupts-extended. Really, either is valid for any binding, but I prefer to just have bindings documented one way.

Documentation/devicetree/bindings/mmc/sdhci-xenon.txt line 11

Quoted 1 line

Documentation/devicetree/bindings/mmc/sdhci-xenon.txt new file +1 -0
+- clocks: array of clocks required for SDHCI.

Missing from the exmaple, and how many clocks and in what order?

Documentation/devicetree/bindings/mmc/sdhci-xenon.txt line 12

Quoted 1 line

Documentation/devicetree/bindings/mmc/sdhci-xenon.txt new file +1 -0
+- bus-width: max bus width in unit of bit, normally should be 1 bit, 4 bits or 8 bits

Isn't this one standard? Just refer back to the common binding.

Documentation/devicetree/bindings/mmc/sdhci-xenon.txt line 16

Quoted 4 lines

Documentation/devicetree/bindings/mmc/sdhci-xenon.txt new file +4 -0
+
+Optional properties:
+- non-removable: whether card is non-removable.
+- auto-cmd12: whether cmd12 is sent automatically.

Seems strange this would be optional for a specific controller. I'd expect either the xenon controller needs this or not.

Quoted 14 lines

Documentation/devicetree/bindings/mmc/sdhci-xenon.txt new file +11 -0

+Example:
+
+sdhci0: sdhci at D8000 {
+	compatible = "marvell,xenon-sdhci";
+	reg = <0xD8000 0x300>;
+	interrupts-extended = <&gic GIC_SPI 26 IRQ_TYPE_LEVEL_HIGH>;
+	non-removable;
+	bus-width = <8>;
+	auto-cmd12;
+	status = "disabled";
+};

Signature · 1 line

2.5.0

Gregory CLEMENT

author

2016-06-14 08:02 UTC #18 lore raw

PATCH 8/12 dt-bindings: mmc: Add Xenon description

Hi Rob,

Quoted 10 lines · Rob Herring

Quoted 9 lines · Gregory CLEMENT
This patch documents the Xenon SDHCI controller that we can find in the Armada 37xx and the Armada 7K and 8K SoCs.
Signed-off-by Gregory CLEMENT <[email protected]>
---
 .../devicetree/bindings/mmc/sdhci-xenon.txt        | 27 ++++++++++++++++++++++
 1 file changed, 27 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mmc/sdhci-xenon.txt

Quoted 12 lines

Documentation/devicetree/bindings/mmc/sdhci-xenon.txt line 4
Quoted 10 lines
Documentation/devicetree/bindings/mmc/sdhci-xenon.txt new file +4 -0
new file mode 100644
index 000000000000..e8d49fda5f12
@@ -0,0 +1,27 @@
+* Marvell sdhci-xenon controller
+
+This file documents differences between the core properties in mmc.txt
+and the properties used by the sdhci-xenon drivers.
Bindings don't document drivers...

Sure I missed this one.

Quoted 6 lines

Documentation/devicetree/bindings/mmc/sdhci-xenon.txt line 7
Quoted 4 lines
Documentation/devicetree/bindings/mmc/sdhci-xenon.txt new file +3 -0
+
+Required properties:
+- compatible: Should be "marvell,xenon-sdhci".
SoC specific compatible strings please. This one can be fallback.

Quoted 8 lines

Documentation/devicetree/bindings/mmc/sdhci-xenon.txt line 10
Quoted 4 lines
Documentation/devicetree/bindings/mmc/sdhci-xenon.txt new file +3 -0
+- reg:
+  one register area for the SDHCI registers.
+- interrupts-extended: interrupt number and attributes
Just use interrupts unless you really have a need for interrupts-extended. Really, either is valid for any binding, but I prefer to just have bindings documented one way.

Quoted 4 lines

Documentation/devicetree/bindings/mmc/sdhci-xenon.txt line 11
Quoted 2 lines
Documentation/devicetree/bindings/mmc/sdhci-xenon.txt new file +1 -0
+- clocks: array of clocks required for SDHCI.
Missing from the exmaple, and how many clocks and in what order?

I need to figure out it.

Quoted 4 lines

Documentation/devicetree/bindings/mmc/sdhci-xenon.txt line 12
Quoted 2 lines
Documentation/devicetree/bindings/mmc/sdhci-xenon.txt new file +1 -0
+- bus-width: max bus width in unit of bit, normally should be 1 bit, 4 bits or 8 bits
Isn't this one standard? Just refer back to the common binding.

OK it will simplify the binding dcumentation.

Quoted 8 lines

Documentation/devicetree/bindings/mmc/sdhci-xenon.txt line 16
Quoted 5 lines
Documentation/devicetree/bindings/mmc/sdhci-xenon.txt new file +4 -0
+
+Optional properties:
+- non-removable: whether card is non-removable.
+- auto-cmd12: whether cmd12 is sent automatically.
Seems strange this would be optional for a specific controller. I'd expect either the xenon controller needs this or not.

I have to check it.

Thanks for the review,

Gregory

Signature · 4 lines

Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

Gregory CLEMENT

author

2016-06-09 07:10 UTC #19 lore raw

PATCH 9/12 mmc: sdhci-xenon: add initial Xenon eMMC driver

In reply to Gregory CLEMENT

From: Victor Gu <[email protected]>

This path adds the driver for the Marvell Xenon eMMC host controller which supports eMMC 5.1, SD 3.0.

However this driver supports only up to eMMC 5.0 since the command queue feature is not included (yet) in the driver.

[gregory.clement at free-electrons.com:
- reformulate commit log
- overload the sdhci_set_uhs_signaling function instead of using a quirk
- fix a kconfig dependency
- remove dead code]

Signed-off-by Victor Gu <[email protected]>

Signed-off-by Gregory CLEMENT <[email protected]>

---
 drivers/mmc/host/Kconfig       |   10 +
 drivers/mmc/host/Makefile      |    1 +
 drivers/mmc/host/sdhci-xenon.c | 1164 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 1175 insertions(+)
 create mode 100644 drivers/mmc/host/sdhci-xenon.c

drivers/mmc/host/Kconfig +10 -0

index 0aa484c10c0a..00b6810e4a6f 100644
@@ -48,6 +48,16 @@ config MMC_SDHCI
 
 	  If unsure, say N.
 
+config MMC_XENON_SDHCI
+	bool "Marvell XENON SD Host Controller"
+	depends on MMC_SDHCI_PLTFM
+	default n
+	help
+	  This selects the Marvell Xenon Host Controller.
+	  Marvell Xenon host controller supports
+	  JEDEC eMMC 5.1, SDIO spec 3.0,
+	  and SD host controller spec 3.0
+
 config MMC_SDHCI_IO_ACCESSORS
 	bool
 	depends on MMC_SDHCI

drivers/mmc/host/Makefile +1 -0

index af918d261ff9..387094031ba9 100644
@@ -76,6 +76,7 @@ obj-$(CONFIG_MMC_SDHCI_IPROC)		+= sdhci-iproc.o
 obj-$(CONFIG_MMC_SDHCI_MSM)		+= sdhci-msm.o
 obj-$(CONFIG_MMC_SDHCI_ST)		+= sdhci-st.o
 obj-$(CONFIG_MMC_SDHCI_MICROCHIP_PIC32)	+= sdhci-pic32.o
+obj-$(CONFIG_MMC_XENON_SDHCI)		+= sdhci-xenon.o
 
 ifeq ($(CONFIG_CB710_DEBUG),y)
 	CFLAGS-cb710-mmc	+= -DDEBUG

drivers/mmc/host/sdhci-xenon.c new file +1164 -0

1,164 lines not on this page · raw message

Signature · 1 line

2.5.0

kbuild test robot

build bot

2016-06-09 13:42 UTC #20 lore raw

[PATCH 09/12] mmc: sdhci-xenon: add initial Xenon eMMC driver

reproduce: wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree…

Hi,

[auto build test ERROR on ulf.hansson-mmc/next]
[also build test ERROR on v4.7-rc2 next-20160609]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Gregory-CLEMENT/Add-SDHCI-Xenon-driver/20160609-151403
base:   https://git.linaro.org/people/ulf.hansson/mmc next
config: ia64-allyesconfig (attached as .config)
compiler: ia64-linux-gcc (GCC) 4.9.0

reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=ia64

All errors (new ones prefixed by >>):

In file included from drivers/mmc/host/sdhci-xenon.c:14:0:

>> arch/ia64/include/uapi/asm/setup.h:7:2: error: unknown type name '__u64'
     __u64 command_line;  /* physical address of command line arguments */
     ^
   arch/ia64/include/uapi/asm/setup.h:8:2: error: unknown type name '__u64'
     __u64 efi_systab;  /* physical address of EFI system table */
     ^
   arch/ia64/include/uapi/asm/setup.h:9:2: error: unknown type name '__u64'
     __u64 efi_memmap;  /* physical address of EFI memory map */
     ^
   arch/ia64/include/uapi/asm/setup.h:10:2: error: unknown type name '__u64'
     __u64 efi_memmap_size;  /* size of EFI memory map */
     ^
   arch/ia64/include/uapi/asm/setup.h:11:2: error: unknown type name '__u64'
     __u64 efi_memdesc_size;  /* size of an EFI memory map descriptor */
     ^
>> arch/ia64/include/uapi/asm/setup.h:12:2: error: unknown type name '__u32'
     __u32 efi_memdesc_version; /* memory descriptor version */
     ^
>> arch/ia64/include/uapi/asm/setup.h:14:3: error: unknown type name '__u16'
      __u16 num_cols; /* number of columns on console output device */
      ^
   arch/ia64/include/uapi/asm/setup.h:15:3: error: unknown type name '__u16'
      __u16 num_rows; /* number of rows on console output device */
      ^
   arch/ia64/include/uapi/asm/setup.h:16:3: error: unknown type name '__u16'
      __u16 orig_x; /* cursor's x position */
      ^
   arch/ia64/include/uapi/asm/setup.h:17:3: error: unknown type name '__u16'
      __u16 orig_y; /* cursor's y position */
      ^
   arch/ia64/include/uapi/asm/setup.h:19:2: error: unknown type name '__u64'
     __u64 fpswa;  /* physical address of the fpswa interface */
     ^
   arch/ia64/include/uapi/asm/setup.h:20:2: error: unknown type name '__u64'
     __u64 initrd_start;
     ^
   arch/ia64/include/uapi/asm/setup.h:21:2: error: unknown type name '__u64'
     __u64 initrd_size;
     ^

vim +/__u64 +7 arch/ia64/include/uapi/asm/setup.h

^1da177e include/asm-ia64/setup.h      Linus Torvalds 2005-04-16   1  #ifndef __IA64_SETUP_H
^1da177e include/asm-ia64/setup.h      Linus Torvalds 2005-04-16   2  #define __IA64_SETUP_H
^1da177e include/asm-ia64/setup.h      Linus Torvalds 2005-04-16   3  
cca97de1 include/asm-ia64/setup.h      Alon Bar-Lev   2007-02-12   4  #define COMMAND_LINE_SIZE	2048
^1da177e include/asm-ia64/setup.h      Linus Torvalds 2005-04-16   5  
c140d879 arch/ia64/include/asm/setup.h David Howells  2012-03-28   6  extern struct ia64_boot_param {
c140d879 arch/ia64/include/asm/setup.h David Howells  2012-03-28  @7  	__u64 command_line;		/* physical address of command line arguments */
c140d879 arch/ia64/include/asm/setup.h David Howells  2012-03-28   8  	__u64 efi_systab;		/* physical address of EFI system table */
c140d879 arch/ia64/include/asm/setup.h David Howells  2012-03-28  @9  	__u64 efi_memmap;		/* physical address of EFI memory map */
c140d879 arch/ia64/include/asm/setup.h David Howells  2012-03-28 @10  	__u64 efi_memmap_size;		/* size of EFI memory map */
c140d879 arch/ia64/include/asm/setup.h David Howells  2012-03-28  11  	__u64 efi_memdesc_size;		/* size of an EFI memory map descriptor */
c140d879 arch/ia64/include/asm/setup.h David Howells  2012-03-28 @12  	__u32 efi_memdesc_version;	/* memory descriptor version */
c140d879 arch/ia64/include/asm/setup.h David Howells  2012-03-28  13  	struct {
c140d879 arch/ia64/include/asm/setup.h David Howells  2012-03-28 @14  		__u16 num_cols;	/* number of columns on console output device */
c140d879 arch/ia64/include/asm/setup.h David Howells  2012-03-28  15  		__u16 num_rows;	/* number of rows on console output device */
c140d879 arch/ia64/include/asm/setup.h David Howells  2012-03-28  16  		__u16 orig_x;	/* cursor's x position */
c140d879 arch/ia64/include/asm/setup.h David Howells  2012-03-28  17  		__u16 orig_y;	/* cursor's y position */

:::::: The code at line 7 was first introduced by commit
:::::: c140d87995b68b428f70635c2e4071e4e8b3256e Disintegrate asm/system.h for IA64

:::::: TO: David Howells <[email protected]> :::::: CC: David Howells <[email protected]>

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation

Gregory CLEMENT

author

2016-06-09 14:33 UTC #21 lore raw

PATCH 9/12 mmc: sdhci-xenon: add initial Xenon eMMC driver

Hi ,

Quoted 59 lines · kbuild test robot

Hi,
[auto build test ERROR on ulf.hansson-mmc/next]
[also build test ERROR on v4.7-rc2 next-20160609]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url:    https://github.com/0day-ci/linux/commits/Gregory-CLEMENT/Add-SDHCI-Xenon-driver/20160609-151403
base:   https://git.linaro.org/people/ulf.hansson/mmc next
config: ia64-allyesconfig (attached as .config)
compiler: ia64-linux-gcc (GCC) 4.9.0
reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=ia64
All errors (new ones prefixed by >>):
In file included from drivers/mmc/host/sdhci-xenon.c:14:0:
Quoted 1 line
Quoted 1 line
arch/ia64/include/uapi/asm/setup.h:7:2: error: unknown type name '__u64'
__u64 command_line; /* physical address of command line arguments */ ^
arch/ia64/include/uapi/asm/setup.h:8:2: error: unknown type name '__u64'
__u64 efi_systab; /* physical address of EFI system table */ ^
arch/ia64/include/uapi/asm/setup.h:9:2: error: unknown type name '__u64'
__u64 efi_memmap; /* physical address of EFI memory map */ ^
arch/ia64/include/uapi/asm/setup.h:10:2: error: unknown type name '__u64'
__u64 efi_memmap_size; /* size of EFI memory map */
^
arch/ia64/include/uapi/asm/setup.h:11:2: error: unknown type name '__u64'
__u64 efi_memdesc_size; /* size of an EFI memory map descriptor */ ^
Quoted 1 line
Quoted 1 line
arch/ia64/include/uapi/asm/setup.h:12:2: error: unknown type name '__u32'
__u32 efi_memdesc_version; /* memory descriptor version */ ^
Quoted 1 line
Quoted 1 line
arch/ia64/include/uapi/asm/setup.h:14:3: error: unknown type name '__u16'
__u16 num_cols; /* number of columns on console output device */ ^
arch/ia64/include/uapi/asm/setup.h:15:3: error: unknown type name '__u16'
__u16 num_rows; /* number of rows on console output device */ ^
arch/ia64/include/uapi/asm/setup.h:16:3: error: unknown type name '__u16'
      __u16 orig_x; /* cursor's x position */
      ^
   arch/ia64/include/uapi/asm/setup.h:17:3: error: unknown type name '__u16'
      __u16 orig_y; /* cursor's y position */
      ^
   arch/ia64/include/uapi/asm/setup.h:19:2: error: unknown type name '__u64'
     __u64 fpswa;  /* physical address of the fpswa interface */
     ^
   arch/ia64/include/uapi/asm/setup.h:20:2: error: unknown type name '__u64'
     __u64 initrd_start;
     ^
   arch/ia64/include/uapi/asm/setup.h:21:2: error: unknown type name '__u64'
     __u64 initrd_size;
     ^

Quoted 1 line

vim +/__u64 +7 arch/ia64/include/uapi/asm/setup.h

it seems that including asm/setup.h was a left over and was not needed, so I remove it.

Gregory

Quoted 29 lines

^1da177e include/asm-ia64/setup.h      Linus Torvalds 2005-04-16   1  #ifndef __IA64_SETUP_H
^1da177e include/asm-ia64/setup.h      Linus Torvalds 2005-04-16   2  #define __IA64_SETUP_H
^1da177e include/asm-ia64/setup.h      Linus Torvalds 2005-04-16   3  
cca97de1 include/asm-ia64/setup.h      Alon Bar-Lev   2007-02-12   4  #define COMMAND_LINE_SIZE	2048
^1da177e include/asm-ia64/setup.h      Linus Torvalds 2005-04-16   5  
c140d879 arch/ia64/include/asm/setup.h David Howells  2012-03-28   6  extern struct ia64_boot_param {
c140d879 arch/ia64/include/asm/setup.h David Howells  2012-03-28  @7  	__u64 command_line;		/* physical address of command line arguments */
c140d879 arch/ia64/include/asm/setup.h David Howells  2012-03-28   8  	__u64 efi_systab;		/* physical address of EFI system table */
c140d879 arch/ia64/include/asm/setup.h David Howells  2012-03-28  @9  	__u64 efi_memmap;		/* physical address of EFI memory map */
c140d879 arch/ia64/include/asm/setup.h David Howells  2012-03-28 @10  	__u64 efi_memmap_size;		/* size of EFI memory map */
c140d879 arch/ia64/include/asm/setup.h David Howells  2012-03-28  11  	__u64 efi_memdesc_size;		/* size of an EFI memory map descriptor */
c140d879 arch/ia64/include/asm/setup.h David Howells  2012-03-28 @12  	__u32 efi_memdesc_version;	/* memory descriptor version */
c140d879 arch/ia64/include/asm/setup.h David Howells  2012-03-28  13  	struct {
c140d879 arch/ia64/include/asm/setup.h David Howells  2012-03-28 @14  		__u16 num_cols;	/* number of columns on console output device */
c140d879 arch/ia64/include/asm/setup.h David Howells  2012-03-28  15  		__u16 num_rows;	/* number of rows on console output device */
c140d879 arch/ia64/include/asm/setup.h David Howells  2012-03-28  16  		__u16 orig_x;	/* cursor's x position */
c140d879 arch/ia64/include/asm/setup.h David Howells  2012-03-28  17  		__u16 orig_y;	/* cursor's y position */

:::::: The code at line 7 was first introduced by commit
:::::: c140d87995b68b428f70635c2e4071e4e8b3256e Disintegrate asm/system.h for IA64

:::::: TO: David Howells <[email protected]> :::::: CC: David Howells <[email protected]>

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation

Signature · 4 lines

Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

kbuild test robot

build bot

2016-06-09 14:20 UTC #22 lore raw

[PATCH 09/12] mmc: sdhci-xenon: add initial Xenon eMMC driver

In reply to Gregory CLEMENT

reproduce:

Hi,

[auto build test ERROR on ulf.hansson-mmc/next]
[also build test ERROR on v4.7-rc2 next-20160609]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Gregory-CLEMENT/Add-SDHCI-Xenon-driver/20160609-151403
base:   https://git.linaro.org/people/ulf.hansson/mmc next
config: x86_64-allmodconfig (attached as .config)
compiler: gcc-6 (Debian 6.1.1-1) 6.1.1 20160430
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64

All errors (new ones prefixed by >>):

drivers/built-in.o: In function `sdhci_xenon_remove':

>> sdhci-xenon.c:(.text+0x311234): undefined reference to `sdhci_remove_host'
>> sdhci-xenon.c:(.text+0x31123c): undefined reference to `sdhci_pltfm_free'
   drivers/built-in.o: In function `sdhci_xenon_delay_adj_test':
>> sdhci-xenon.c:(.text+0x31213b): undefined reference to `card_alive'
   drivers/built-in.o: In function `sdhci_xenon_set_uhs_signaling':
>> sdhci-xenon.c:(.text+0x312456): undefined reference to `sdhci_set_uhs_signaling'
   drivers/built-in.o: In function `sdhci_xenon_probe':
>> sdhci-xenon.c:(.text+0x31258c): undefined reference to `sdhci_pltfm_init'
>> sdhci-xenon.c:(.text+0x3125d3): undefined reference to `mmc_of_parse'
>> sdhci-xenon.c:(.text+0x312651): undefined reference to `sdhci_get_of_property'
>> sdhci-xenon.c:(.text+0x312840): undefined reference to `sdhci_add_host'
   sdhci-xenon.c:(.text+0x312904): undefined reference to `sdhci_pltfm_free'
   drivers/built-in.o: In function `sdhci_xenon_reset':
>> sdhci-xenon.c:(.text+0x313c11): undefined reference to `sdhci_reset'
>> drivers/built-in.o:(.data+0x1d7df0): undefined reference to `sdhci_pltfm_pmops'
>> drivers/built-in.o:(.data+0x1d7eb0): undefined reference to `sdhci_set_clock'
>> drivers/built-in.o:(.data+0x1d7ef0): undefined reference to `sdhci_set_bus_width'

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: .config.gz
Type: application/octet-stream
Size: 54760 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160609/31c0af2c/attachment-0001.obj>

Gregory CLEMENT

author

2016-06-09 14:54 UTC #23 lore raw

PATCH 9/12 mmc: sdhci-xenon: add initial Xenon eMMC driver

Hi,

I added Arnd because he seems to have a good knoweldge of the Kconfig stuff.

Quoted 22 lines · kbuild test robot

Hi,
[auto build test ERROR on ulf.hansson-mmc/next]
[also build test ERROR on v4.7-rc2 next-20160609]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url:    https://github.com/0day-ci/linux/commits/Gregory-CLEMENT/Add-SDHCI-Xenon-driver/20160609-151403
base:   https://git.linaro.org/people/ulf.hansson/mmc next
config: x86_64-allmodconfig (attached as .config)
compiler: gcc-6 (Debian 6.1.1-1) 6.1.1 20160430
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 
All errors (new ones prefixed by >>):
drivers/built-in.o: In function `sdhci_xenon_remove':
Quoted 2 lines
Quoted 2 lines
sdhci-xenon.c:(.text+0x311234): undefined reference to sdhci_remove_host' sdhci-xenon.c:(.text+0x31123c): undefined reference to sdhci_pltfm_free'
drivers/built-in.o: In function `sdhci_xenon_delay_adj_test':
Quoted 1 line
Quoted 1 line
sdhci-xenon.c:(.text+0x31213b): undefined reference to `card_alive'
drivers/built-in.o: In function `sdhci_xenon_set_uhs_signaling':

Quoted 12 lines

Quoted 1 line
Quoted 1 line
sdhci-xenon.c:(.text+0x312456): undefined reference to `sdhci_set_uhs_signaling'
drivers/built-in.o: In function `sdhci_xenon_probe':
Quoted 4 lines
Quoted 4 lines
sdhci-xenon.c:(.text+0x31258c): undefined reference to sdhci_pltfm_init' sdhci-xenon.c:(.text+0x3125d3): undefined reference to mmc_of_parse' sdhci-xenon.c:(.text+0x312651): undefined reference to sdhci_get_of_property' sdhci-xenon.c:(.text+0x312840): undefined reference to sdhci_add_host'
sdhci-xenon.c:(.text+0x312904): undefined reference to sdhci_pltfm_free' drivers/built-in.o: In function sdhci_xenon_reset':
Quoted 4 lines
Quoted 4 lines
sdhci-xenon.c:(.text+0x313c11): undefined reference to sdhci_reset' drivers/built-in.o:(.data+0x1d7df0): undefined reference to sdhci_pltfm_pmops' drivers/built-in.o:(.data+0x1d7eb0): undefined reference to sdhci_set_clock' drivers/built-in.o:(.data+0x1d7ef0): undefined reference to sdhci_set_bus_width'

These symbols are part of the drivers/mmc/host/sdhci.c which is build when CONFIG_MMC_SDHCI is enabled. However CONFIG_MMC_XENON_SDHCI depends on CONFIG_MMC_SDHCI_PLTFM which depends on CONFIG_MMC_SDHCI.

So I wonder how we can have a sition with sdhci-xenon.c built but not sdhci.c.

Any idea?

Thanks,

Gregory

Quoted 5 lines

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation

Signature · 4 lines

Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

Arnd Bergmann

2016-06-09 21:49 UTC #24 lore raw

PATCH 9/12 mmc: sdhci-xenon: add initial Xenon eMMC driver

Quoted 20 lines · CEST Gregory CLEMENT

Quoted 7 lines
drivers/built-in.o: In function `sdhci_xenon_remove':
Quoted 2 lines
Quoted 2 lines
sdhci-xenon.c:(.text+0x311234): undefined reference to sdhci_remove_host' sdhci-xenon.c:(.text+0x31123c): undefined reference to sdhci_pltfm_free'
drivers/built-in.o: In function `sdhci_xenon_delay_adj_test':
Quoted 1 line
Quoted 1 line
sdhci-xenon.c:(.text+0x31213b): undefined reference to `card_alive'
drivers/built-in.o: In function `sdhci_xenon_set_uhs_signaling':
Quoted 12 lines
Quoted 1 line
Quoted 1 line
sdhci-xenon.c:(.text+0x312456): undefined reference to `sdhci_set_uhs_signaling'
drivers/built-in.o: In function `sdhci_xenon_probe':
Quoted 4 lines
Quoted 4 lines
sdhci-xenon.c:(.text+0x31258c): undefined reference to sdhci_pltfm_init' sdhci-xenon.c:(.text+0x3125d3): undefined reference to mmc_of_parse' sdhci-xenon.c:(.text+0x312651): undefined reference to sdhci_get_of_property' sdhci-xenon.c:(.text+0x312840): undefined reference to sdhci_add_host'
sdhci-xenon.c:(.text+0x312904): undefined reference to sdhci_pltfm_free' drivers/built-in.o: In function sdhci_xenon_reset':
Quoted 4 lines
Quoted 4 lines
sdhci-xenon.c:(.text+0x313c11): undefined reference to sdhci_reset' drivers/built-in.o:(.data+0x1d7df0): undefined reference to sdhci_pltfm_pmops' drivers/built-in.o:(.data+0x1d7eb0): undefined reference to sdhci_set_clock' drivers/built-in.o:(.data+0x1d7ef0): undefined reference to sdhci_set_bus_width'

Quoted 9 lines

These symbols are part of the drivers/mmc/host/sdhci.c which is build when CONFIG_MMC_SDHCI is enabled. However CONFIG_MMC_XENON_SDHCI depends on CONFIG_MMC_SDHCI_PLTFM which depends on CONFIG_MMC_SDHCI.
So I wonder how we can have a sition with sdhci-xenon.c built but not sdhci.c.
Any idea?

CONFIG_MMC_SDHCI is built as a module, but CONFIG_MMC_XENON_SDHCI is a "bool" option, so it can only be built-in.

It should work if you make it a 'tristate' instead, so that Kconfig can handle the dependency correctly.

Arnd

Adrian Hunter

maintainer

2016-06-14 06:52 UTC #25 lore raw

PATCH 9/12 mmc: sdhci-xenon: add initial Xenon eMMC driver

In reply to Gregory CLEMENT

Quoted 23 lines · Gregory CLEMENT

From: Victor Gu <[email protected]>
This path adds the driver for the Marvell Xenon eMMC host controller which supports eMMC 5.1, SD 3.0.
However this driver supports only up to eMMC 5.0 since the command queue feature is not included (yet) in the driver.
[gregory.clement at free-electrons.com:
- reformulate commit log
- overload the sdhci_set_uhs_signaling function instead of using a quirk
- fix a kconfig dependency
- remove dead code]
Signed-off-by Victor Gu <[email protected]>
Signed-off-by Gregory CLEMENT <[email protected]>
---
 drivers/mmc/host/Kconfig       |   10 +
 drivers/mmc/host/Makefile      |    1 +
 drivers/mmc/host/sdhci-xenon.c | 1164 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 1175 insertions(+)
 create mode 100644 drivers/mmc/host/sdhci-xenon.c

<SNIP>

Quoted 5 lines

drivers/mmc/host/sdhci-xenon.c new file +0 -0
new file mode 100644
index 000000000000..43c06db95872

<SNIP>

Quoted 7 lines

drivers/mmc/host/sdhci-xenon.c new file +7 -0

+static int sdhci_xenon_delay_adj_test(struct sdhci_host *host,
+		struct mmc_card *card, unsigned int delay,
+		bool invert, bool phase)
+{
+	int ret;
+	struct mmc_card *oldcard;
+

drivers/mmc/host/sdhci-xenon.c line 485

Quoted 10 lines

drivers/mmc/host/sdhci-xenon.c new file +10 -0

+	sdhci_xenon_set_fix_delay(host, delay, invert, phase);
+
+	/*
+	 * If the card is not yet associated to the host, we
+	 * temporally do it
+	 */
+	oldcard = card->host->card;
+	if (!oldcard)
+		card->host->card = card;
+	ret =  card_alive(card);

This looks like an abuse of bus_ops->alive(). You will need to get agreement with Ulf how to handle this. I will wait for Ulf's comments before reviewing this patch set further.

Quoted 11 lines

drivers/mmc/host/sdhci-xenon.c new file +11 -0

+	card->host->card = oldcard;
+	if (ret) {
+		pr_debug("Xenon failed when sampling fix delay %d, inverted %d, phase %d\n",
+			 delay, invert, phase);
+		return -EIO;
+	}
+
+	pr_debug("Xenon succeeded when sampling fixed delay %d, inverted %d, phase %d\n",
+		 delay, invert, phase);
+	return 0;
+}

Gregory CLEMENT

author

2016-06-14 08:19 UTC #26 lore raw

PATCH 9/12 mmc: sdhci-xenon: add initial Xenon eMMC driver

Hi Adrian,

Quoted 48 lines · Adrian Hunter

Quoted 23 lines · Gregory CLEMENT
From: Victor Gu <[email protected]>
This path adds the driver for the Marvell Xenon eMMC host controller which supports eMMC 5.1, SD 3.0.
However this driver supports only up to eMMC 5.0 since the command queue feature is not included (yet) in the driver.
[gregory.clement at free-electrons.com:
- reformulate commit log
- overload the sdhci_set_uhs_signaling function instead of using a quirk
- fix a kconfig dependency
- remove dead code]
Signed-off-by Victor Gu <[email protected]>
Signed-off-by Gregory CLEMENT <[email protected]>
---
 drivers/mmc/host/Kconfig       |   10 +
 drivers/mmc/host/Makefile      |    1 +
 drivers/mmc/host/sdhci-xenon.c | 1164 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 1175 insertions(+)
 create mode 100644 drivers/mmc/host/sdhci-xenon.c
<SNIP>
Quoted 5 lines
drivers/mmc/host/sdhci-xenon.c new file +0 -0
new file mode 100644
index 000000000000..43c06db95872
<SNIP>
Quoted 13 lines
drivers/mmc/host/sdhci-xenon.c new file +13 -0
+static int sdhci_xenon_delay_adj_test(struct sdhci_host *host,
+		struct mmc_card *card, unsigned int delay,
+		bool invert, bool phase)
+{
+	int ret;
+	struct mmc_card *oldcard;
+
+	sdhci_xenon_set_fix_delay(host, delay, invert, phase);
+
+	/*
+	 * If the card is not yet associated to the host, we
+	 * temporally do it
+	 */

Quoted 8 lines

drivers/mmc/host/sdhci-xenon.c line 485
Quoted 4 lines
drivers/mmc/host/sdhci-xenon.c new file +4 -0
+	oldcard = card->host->card;
+	if (!oldcard)
+		card->host->card = card;
+	ret =  card_alive(card);
This looks like an abuse of bus_ops->alive(). You will need to get agreement with Ulf how to handle this. I will wait for Ulf's comments before reviewing this patch set further.

Actually I modified this part of the driver to use bus_ops->alive(). Initially, the alive() code was more or less copied and paste from the mmc core with ugly include from mmc_ops.h and sdio_ops.h to make it work. I find it cleaner to directly access the alive() function.

Could you explain why it is an abuse of bus_ops->alive()?

Thanks,

Gregory

Quoted 13 lines

Quoted 11 lines

drivers/mmc/host/sdhci-xenon.c new file +11 -0

+	card->host->card = oldcard;
+	if (ret) {
+		pr_debug("Xenon failed when sampling fix delay %d, inverted %d, phase %d\n",
+			 delay, invert, phase);
+		return -EIO;
+	}
+
+	pr_debug("Xenon succeeded when sampling fixed delay %d, inverted %d, phase %d\n",
+		 delay, invert, phase);
+	return 0;
+}

Signature · 4 lines

Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

Adrian Hunter

maintainer

2016-06-14 08:36 UTC #27 lore raw

PATCH 9/12 mmc: sdhci-xenon: add initial Xenon eMMC driver

Quoted 61 lines · Gregory CLEMENT

Hi Adrian,
Quoted 48 lines · Adrian Hunter
Quoted 23 lines · Gregory CLEMENT
From: Victor Gu <[email protected]>
This path adds the driver for the Marvell Xenon eMMC host controller which supports eMMC 5.1, SD 3.0.
However this driver supports only up to eMMC 5.0 since the command queue feature is not included (yet) in the driver.
[gregory.clement at free-electrons.com:
- reformulate commit log
- overload the sdhci_set_uhs_signaling function instead of using a quirk
- fix a kconfig dependency
- remove dead code]
Signed-off-by Victor Gu <[email protected]>
Signed-off-by Gregory CLEMENT <[email protected]>
---
 drivers/mmc/host/Kconfig       |   10 +
 drivers/mmc/host/Makefile      |    1 +
 drivers/mmc/host/sdhci-xenon.c | 1164 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 1175 insertions(+)
 create mode 100644 drivers/mmc/host/sdhci-xenon.c
<SNIP>
Quoted 5 lines
drivers/mmc/host/sdhci-xenon.c new file +0 -0
new file mode 100644
index 000000000000..43c06db95872
<SNIP>
Quoted 13 lines
drivers/mmc/host/sdhci-xenon.c new file +13 -0
+static int sdhci_xenon_delay_adj_test(struct sdhci_host *host,
+		struct mmc_card *card, unsigned int delay,
+		bool invert, bool phase)
+{
+	int ret;
+	struct mmc_card *oldcard;
+
+	sdhci_xenon_set_fix_delay(host, delay, invert, phase);
+
+	/*
+	 * If the card is not yet associated to the host, we
+	 * temporally do it
+	 */
Quoted 8 lines
drivers/mmc/host/sdhci-xenon.c line 485
Quoted 4 lines
drivers/mmc/host/sdhci-xenon.c new file +4 -0
+	oldcard = card->host->card;
+	if (!oldcard)
+		card->host->card = card;
+	ret =  card_alive(card);
This looks like an abuse of bus_ops->alive(). You will need to get agreement with Ulf how to handle this. I will wait for Ulf's comments before reviewing this patch set further.

Quoted 7 lines

Actually I modified this part of the driver to use bus_ops->alive(). Initially, the alive() code was more or less copied and paste from the mmc core with ugly include from mmc_ops.h and sdio_ops.h to make it work. I find it cleaner to directly access the alive() function.
Could you explain why it is an abuse of bus_ops->alive()?

bus_ops->alive() is used to determine if the card is present and able to respond. You seem to be using it for a different purpose. I will wait for Ulf's comments.

Ulf Hansson

2016-06-22 11:04 UTC #28 lore raw

PATCH 9/12 mmc: sdhci-xenon: add initial Xenon eMMC driver

Quoted 62 lines · Adrian Hunter

Quoted 61 lines · Gregory CLEMENT
Hi Adrian,
Quoted 56 lines · Adrian Hunter
Quoted 23 lines · Gregory CLEMENT
From: Victor Gu <[email protected]>
This path adds the driver for the Marvell Xenon eMMC host controller which supports eMMC 5.1, SD 3.0.
However this driver supports only up to eMMC 5.0 since the command queue feature is not included (yet) in the driver.
[gregory.clement at free-electrons.com:
- reformulate commit log
- overload the sdhci_set_uhs_signaling function instead of using a quirk
- fix a kconfig dependency
- remove dead code]
Signed-off-by Victor Gu <[email protected]>
Signed-off-by Gregory CLEMENT <[email protected]>
---
 drivers/mmc/host/Kconfig       |   10 +
 drivers/mmc/host/Makefile      |    1 +
 drivers/mmc/host/sdhci-xenon.c | 1164 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 1175 insertions(+)
 create mode 100644 drivers/mmc/host/sdhci-xenon.c
<SNIP>
Quoted 5 lines
drivers/mmc/host/sdhci-xenon.c new file +0 -0
new file mode 100644
index 000000000000..43c06db95872
<SNIP>
Quoted 7 lines
drivers/mmc/host/sdhci-xenon.c new file +7 -0
+static int sdhci_xenon_delay_adj_test(struct sdhci_host *host,
+           struct mmc_card *card, unsigned int delay,
+           bool invert, bool phase)
+{
+   int ret;
+   struct mmc_card *oldcard;
+
drivers/mmc/host/sdhci-xenon.c line 485
Quoted 10 lines
drivers/mmc/host/sdhci-xenon.c new file +10 -0
+   sdhci_xenon_set_fix_delay(host, delay, invert, phase);
+
+   /*
+    * If the card is not yet associated to the host, we
+    * temporally do it
+    */
+   oldcard = card->host->card;
+   if (!oldcard)
+           card->host->card = card;
+   ret =  card_alive(card);
This looks like an abuse of bus_ops->alive(). You will need to get agreement with Ulf how to handle this. I will wait for Ulf's comments before reviewing this patch set further.

Quoted 7 lines

Quoted 7 lines
Actually I modified this part of the driver to use bus_ops->alive(). Initially, the alive() code was more or less copied and paste from the mmc core with ugly include from mmc_ops.h and sdio_ops.h to make it work. I find it cleaner to directly access the alive() function.
Could you explain why it is an abuse of bus_ops->alive()?

Because it's an internal callback to be used only by the mmc core.

Quoted 5 lines

bus_ops->alive() is used to determine if the card is present and able to respond. You seem to be using it for a different purpose. I will wait for Ulf's comments.

I looked briefly at your code, which uses the ->alive() ops. Indeed it's a workaround which I cannot accept.

Could you elaborate why you need to send a CMD13 from your host driver?

It's the responsibility of the mmc core to know/implement the (e)MMC/SD/SDIO protocols. Is there something missing in core to be able to support your host driver?

Kind regards
Uffe

Gregory CLEMENT

author

2016-06-09 07:10 UTC #29 lore raw

PATCH 10/12 arm64: dts: marvell: add eMMC support for Armada 37xx

In reply to Gregory CLEMENT

Add the eMMC support for Armada 37xx SoC and enable it in the Armada 3720 DB board.

Signed-off-by Gregory CLEMENT <[email protected]>

---
 arch/arm64/boot/dts/marvell/armada-3720-db.dts | 6 ++++++
 arch/arm64/boot/dts/marvell/armada-37xx.dtsi   | 9 +++++++++
 2 files changed, 15 insertions(+)

arch/arm64/boot/dts/marvell/armada-3720-db.dts +6 -0

index 86110a6ae330..569756924e3e 100644
@@ -72,6 +72,12 @@
 	status = "okay";
 };
 
+&sdhci0 {
+	non-removable;
+	bus-width = <8>;
+	status = "okay";
+};
+
 /* CON31 */
 &usb3 {
 	status = "okay";

arch/arm64/boot/dts/marvell/armada-37xx.dtsi +9 -0

index 9e2efb882983..5cc8b3f5ef51 100644
@@ -126,6 +126,15 @@
 				};
 			};
 
+			sdhci0: sdhci at d8000 {
+				compatible = "marvell,xenon-sdhci";
+				reg = <0xd8000 0x300>;
+				interrupts = <GIC_SPI 26 IRQ_TYPE_LEVEL_HIGH>;
+
+				auto-cmd12;
+				status = "disabled";
+			};
+
 			sata: sata at e0000 {
 				compatible = "marvell,armada-3700-ahci";
 				reg = <0xe0000 0x2000>;

Signature · 1 line

2.5.0

Gregory CLEMENT

author

2016-06-09 07:10 UTC #30 lore raw

PATCH 11/12 arm64: dts: marvell: add SDIO support for Armada 37xx

In reply to Gregory CLEMENT

Add the SDIO support for Armada 37xx SoC and enable it in the Armada 3720 DB board.

Signed-off-by Gregory CLEMENT <[email protected]>

---
 arch/arm64/boot/dts/marvell/armada-3720-db.dts | 6 ++++++
 arch/arm64/boot/dts/marvell/armada-37xx.dtsi   | 8 ++++++++
 2 files changed, 14 insertions(+)

arch/arm64/boot/dts/marvell/armada-3720-db.dts +6 -0

index 569756924e3e..5352d101562d 100644
@@ -78,6 +78,12 @@
 	status = "okay";
 };
 
+&sdhci1 {
+	broken-cd;
+	bus-width = <4>;
+	status = "okay";
+};
+
 /* CON31 */
 &usb3 {
 	status = "okay";

arch/arm64/boot/dts/marvell/armada-37xx.dtsi +8 -0

index 5cc8b3f5ef51..7803ca135d37 100644
@@ -126,6 +126,14 @@
 				};
 			};
 
+			sdhci1: sdhci at d0000 {
+				compatible = "marvell,xenon-sdhci";
+				reg = <0xd0000 0x300>;
+				interrupts = <GIC_SPI 25 IRQ_TYPE_LEVEL_HIGH>;
+				auto-cmd12;
+				status = "disabled";
+			};
+
 			sdhci0: sdhci at d8000 {
 				compatible = "marvell,xenon-sdhci";
 				reg = <0xd8000 0x300>;

Signature · 1 line

2.5.0

Gregory CLEMENT

author

2016-06-09 07:10 UTC #31 lore raw

PATCH 12/12 arm64: defconfig: Add Xenon sdhci

In reply to Gregory CLEMENT

Enable Xenon sdhci found on Armada 37xx and Armada 7K/8K SoCs

Signed-off-by Gregory CLEMENT <[email protected]>

---
 arch/arm64/configs/defconfig | 1 +
 1 file changed, 1 insertion(+)

arch/arm64/configs/defconfig +1 -0

index fd2d74d0491e..2fe02461be55 100644
@@ -245,6 +245,7 @@ CONFIG_MMC=y
 CONFIG_MMC_BLOCK_MINORS=32
 CONFIG_MMC_ARMMMCI=y
 CONFIG_MMC_SDHCI=y
+CONFIG_MMC_XENON_SDHCI=y
 CONFIG_MMC_SDHCI_PLTFM=y
 CONFIG_MMC_SDHCI_OF_ESDHC=y
 CONFIG_MMC_SDHCI_TEGRA=y

Signature · 1 line

2.5.0