The series first cleans up a common pattern, which is ultimately needed
to integrate the regulator with bcm2711's sdhci-iproc. It then
introduces the relevant device-tree changes.
---
Changes since v1:
- Use helper function istead of quirk
- Add GPIO label
Nicolas Saenz Julienne (11):
mmc: sdhci: Introduce sdhci_set_power_and_bus_voltage()
mmc: sdhci: arasan: Use sdhci_set_power_and_voltage()
mmc: sdhci: milbeaut: Use sdhci_set_power_and_voltage()
mmc: sdhci: at91: Use sdhci_set_power_and_voltage()
mmc: sdhci: pxav3: Use sdhci_set_power_and_voltage()
mmc: sdhci: xenon: Use sdhci_set_power_and_voltage()
mmc: sdhci: am654: Use sdhci_set_power_and_voltage()
mmc: sdhci: Unexport sdhci_set_power_noreg()
mmc: sdhci: iproc: Add custom set_power() callback for bcm2711
ARM: dts: bcm2711: Update expgpio's GPIO labels
ARM: dts: bcm2711: Add vmmc regulator in emmc2
arch/arm/boot/dts/bcm2711-rpi-4-b.dts | 13 ++++++++++++-
drivers/mmc/host/sdhci-iproc.c | 17 ++++++++++++++++-
drivers/mmc/host/sdhci-milbeaut.c | 13 +------------
drivers/mmc/host/sdhci-of-arasan.c | 15 ++-------------
drivers/mmc/host/sdhci-of-at91.c | 18 +-----------------
drivers/mmc/host/sdhci-pxav3.c | 20 +-------------------
drivers/mmc/host/sdhci-xenon.c | 20 +-------------------
drivers/mmc/host/sdhci.c | 24 +++++++++++++++++++++---
drivers/mmc/host/sdhci.h | 5 +++--
drivers/mmc/host/sdhci_am654.c | 17 +++--------------
10 files changed, 61 insertions(+), 101 deletions(-)
--
2.25.1
The sdhci core provides a helper function with the same functionality as
this controller's set_power() callback. Use it instead.
Signed-off-by: Nicolas Saenz Julienne <[email protected]>
---
drivers/mmc/host/sdhci-milbeaut.c | 13 +------------
1 file changed, 1 insertion(+), 12 deletions(-)
diff --git a/drivers/mmc/host/sdhci-milbeaut.c b/drivers/mmc/host/sdhci-milbeaut.c
index 92f30a1db435..4e7cc0680f94 100644
--- a/drivers/mmc/host/sdhci-milbeaut.c
+++ b/drivers/mmc/host/sdhci-milbeaut.c
@@ -121,17 +121,6 @@ static void sdhci_milbeaut_reset(struct sdhci_host *host, u8 mask)
}
}
-static void sdhci_milbeaut_set_power(struct sdhci_host *host,
- unsigned char mode, unsigned short vdd)
-{
- if (!IS_ERR(host->mmc->supply.vmmc)) {
- struct mmc_host *mmc = host->mmc;
-
- mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd);
- }
- sdhci_set_power_noreg(host, mode, vdd);
-}
-
static const struct sdhci_ops sdhci_milbeaut_ops = {
.voltage_switch = sdhci_milbeaut_soft_voltage_switch,
.get_min_clock = sdhci_milbeaut_get_min_clock,
@@ -139,7 +128,7 @@ static const struct sdhci_ops sdhci_milbeaut_ops = {
.set_clock = sdhci_set_clock,
.set_bus_width = sdhci_set_bus_width,
.set_uhs_signaling = sdhci_set_uhs_signaling,
- .set_power = sdhci_milbeaut_set_power,
+ .set_power = sdhci_set_power_and_bus_voltage,
};
static void sdhci_milbeaut_bridge_reset(struct sdhci_host *host,
--
2.25.1
The sdhci core provides a helper function with the same functionality as
this controller's set_power() callback. Use it instead.
Signed-off-by: Nicolas Saenz Julienne <[email protected]>
---
drivers/mmc/host/sdhci_am654.c | 17 +++--------------
1 file changed, 3 insertions(+), 14 deletions(-)
diff --git a/drivers/mmc/host/sdhci_am654.c b/drivers/mmc/host/sdhci_am654.c
index 3afea580fbea..c70647489bbd 100644
--- a/drivers/mmc/host/sdhci_am654.c
+++ b/drivers/mmc/host/sdhci_am654.c
@@ -208,17 +208,6 @@ static void sdhci_j721e_4bit_set_clock(struct sdhci_host *host,
sdhci_set_clock(host, clock);
}
-static void sdhci_am654_set_power(struct sdhci_host *host, unsigned char mode,
- unsigned short vdd)
-{
- if (!IS_ERR(host->mmc->supply.vmmc)) {
- struct mmc_host *mmc = host->mmc;
-
- mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd);
- }
- sdhci_set_power_noreg(host, mode, vdd);
-}
-
static void sdhci_am654_write_b(struct sdhci_host *host, u8 val, int reg)
{
unsigned char timing = host->mmc->ios.timing;
@@ -274,7 +263,7 @@ static struct sdhci_ops sdhci_am654_ops = {
.get_timeout_clock = sdhci_pltfm_clk_get_max_clock,
.set_uhs_signaling = sdhci_set_uhs_signaling,
.set_bus_width = sdhci_set_bus_width,
- .set_power = sdhci_am654_set_power,
+ .set_power = sdhci_set_power_and_bus_voltage,
.set_clock = sdhci_am654_set_clock,
.write_b = sdhci_am654_write_b,
.irq = sdhci_am654_cqhci_irq,
@@ -297,7 +286,7 @@ static struct sdhci_ops sdhci_j721e_8bit_ops = {
.get_timeout_clock = sdhci_pltfm_clk_get_max_clock,
.set_uhs_signaling = sdhci_set_uhs_signaling,
.set_bus_width = sdhci_set_bus_width,
- .set_power = sdhci_am654_set_power,
+ .set_power = sdhci_set_power_and_bus_voltage,
.set_clock = sdhci_am654_set_clock,
.write_b = sdhci_am654_write_b,
.irq = sdhci_am654_cqhci_irq,
@@ -320,7 +309,7 @@ static struct sdhci_ops sdhci_j721e_4bit_ops = {
.get_timeout_clock = sdhci_pltfm_clk_get_max_clock,
.set_uhs_signaling = sdhci_set_uhs_signaling,
.set_bus_width = sdhci_set_bus_width,
- .set_power = sdhci_am654_set_power,
+ .set_power = sdhci_set_power_and_bus_voltage,
.set_clock = sdhci_j721e_4bit_set_clock,
.write_b = sdhci_am654_write_b,
.irq = sdhci_am654_cqhci_irq,
--
2.25.1
The 6th line of the GPIO expander is used to power the board's SD card.
Signed-off-by: Nicolas Saenz Julienne <[email protected]>
---
arch/arm/boot/dts/bcm2711-rpi-4-b.dts | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
index 1d4b589fe233..b0ea8233b636 100644
--- a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
+++ b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
@@ -68,7 +68,7 @@ expgpio: gpio {
"GLOBAL_RESET",
"VDD_SD_IO_SEL",
"CAM_GPIO",
- "",
+ "SD_PWR_ON",
"";
status = "okay";
};
--
2.25.1
The SD card power can be controlled trough a pin routed into the board's
external GPIO expander. Turn that into a regulator and provide it to
emmc2.
Signed-off-by: Nicolas Saenz Julienne <[email protected]>
---
arch/arm/boot/dts/bcm2711-rpi-4-b.dts | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
index b0ea8233b636..a2da058396fe 100644
--- a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
+++ b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
@@ -55,6 +55,16 @@ sd_io_1v8_reg: sd_io_1v8_reg {
3300000 0x0>;
status = "okay";
};
+
+ sd_vcc_reg: sd_vcc_reg {
+ compatible = "regulator-fixed";
+ regulator-name = "vcc-sd";
+ regulator-min-microvolt = <3300000>;
+ regulator-max-microvolt = <3300000>;
+ regulator-boot-on;
+ enable-active-high;
+ gpio = <&expgpio 6 GPIO_ACTIVE_HIGH>;
+ };
};
&firmware {
@@ -173,6 +183,7 @@ brcmf: wifi@1 {
/* EMMC2 is used to drive the SD card */
&emmc2 {
vqmmc-supply = <&sd_io_1v8_reg>;
+ vmmc-supply = <&sd_vcc_reg>;
broken-cd;
status = "okay";
};
--
2.25.1
The sdhci core provides a helper function with the same functionality as
this controller's set_power() callback. Use it instead.
Signed-off-by: Nicolas Saenz Julienne <[email protected]>
---
drivers/mmc/host/sdhci-xenon.c | 20 +-------------------
1 file changed, 1 insertion(+), 19 deletions(-)
diff --git a/drivers/mmc/host/sdhci-xenon.c b/drivers/mmc/host/sdhci-xenon.c
index 1dea1ba66f7b..1e9a7a76f2ba 100644
--- a/drivers/mmc/host/sdhci-xenon.c
+++ b/drivers/mmc/host/sdhci-xenon.c
@@ -213,24 +213,6 @@ static void xenon_set_uhs_signaling(struct sdhci_host *host,
sdhci_writew(host, ctrl_2, SDHCI_HOST_CONTROL2);
}
-static void xenon_set_power(struct sdhci_host *host, unsigned char mode,
- unsigned short vdd)
-{
- struct mmc_host *mmc = host->mmc;
- u8 pwr = host->pwr;
-
- sdhci_set_power_noreg(host, mode, vdd);
-
- if (host->pwr == pwr)
- return;
-
- if (host->pwr == 0)
- vdd = 0;
-
- if (!IS_ERR(mmc->supply.vmmc))
- mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd);
-}
-
static void xenon_voltage_switch(struct sdhci_host *host)
{
/* Wait for 5ms after set 1.8V signal enable bit */
@@ -240,7 +222,7 @@ static void xenon_voltage_switch(struct sdhci_host *host)
static const struct sdhci_ops sdhci_xenon_ops = {
.voltage_switch = xenon_voltage_switch,
.set_clock = sdhci_set_clock,
- .set_power = xenon_set_power,
+ .set_power = sdhci_set_power_and_bus_voltage,
.set_bus_width = sdhci_set_bus_width,
.reset = xenon_reset,
.set_uhs_signaling = xenon_set_uhs_signaling,
--
2.25.1
There are no users left and, ideally, this isn't a function that should
be called outside of core sdhci code.
Signed-off-by: Nicolas Saenz Julienne <[email protected]>
---
drivers/mmc/host/sdhci.c | 5 ++---
drivers/mmc/host/sdhci.h | 2 --
2 files changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 6ed11f9554e8..37f0107fc276 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1926,8 +1926,8 @@ static void sdhci_set_power_reg(struct sdhci_host *host, unsigned char mode,
sdhci_writeb(host, 0, SDHCI_POWER_CONTROL);
}
-void sdhci_set_power_noreg(struct sdhci_host *host, unsigned char mode,
- unsigned short vdd)
+static void sdhci_set_power_noreg(struct sdhci_host *host, unsigned char mode,
+ unsigned short vdd)
{
u8 pwr = 0;
@@ -1998,7 +1998,6 @@ void sdhci_set_power_noreg(struct sdhci_host *host, unsigned char mode,
mdelay(10);
}
}
-EXPORT_SYMBOL_GPL(sdhci_set_power_noreg);
void sdhci_set_power(struct sdhci_host *host, unsigned char mode,
unsigned short vdd)
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 1be7c18264cd..9bb84b0f87d5 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -775,8 +775,6 @@ void sdhci_set_power(struct sdhci_host *host, unsigned char mode,
void sdhci_set_power_and_bus_voltage(struct sdhci_host *host,
unsigned char mode,
unsigned short vdd);
-void sdhci_set_power_noreg(struct sdhci_host *host, unsigned char mode,
- unsigned short vdd);
void sdhci_request(struct mmc_host *mmc, struct mmc_request *mrq);
void sdhci_set_bus_width(struct sdhci_host *host, int width);
void sdhci_reset(struct sdhci_host *host, u8 mask);
--
2.25.1
The sdhci core provides a helper function with the same functionality as
this controller's set_power() callback. Use it instead.
Signed-off-by: Nicolas Saenz Julienne <[email protected]>
---
drivers/mmc/host/sdhci-of-at91.c | 18 +-----------------
1 file changed, 1 insertion(+), 17 deletions(-)
diff --git a/drivers/mmc/host/sdhci-of-at91.c b/drivers/mmc/host/sdhci-of-at91.c
index ab2bd314a390..564a606f6729 100644
--- a/drivers/mmc/host/sdhci-of-at91.c
+++ b/drivers/mmc/host/sdhci-of-at91.c
@@ -101,22 +101,6 @@ static void sdhci_at91_set_clock(struct sdhci_host *host, unsigned int clock)
sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
}
-/*
- * In this specific implementation of the SDHCI controller, the power register
- * needs to have a valid voltage set even when the power supply is managed by
- * an external regulator.
- */
-static void sdhci_at91_set_power(struct sdhci_host *host, unsigned char mode,
- unsigned short vdd)
-{
- if (!IS_ERR(host->mmc->supply.vmmc)) {
- struct mmc_host *mmc = host->mmc;
-
- mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd);
- }
- sdhci_set_power_noreg(host, mode, vdd);
-}
-
static void sdhci_at91_set_uhs_signaling(struct sdhci_host *host,
unsigned int timing)
{
@@ -145,7 +129,7 @@ static const struct sdhci_ops sdhci_at91_sama5d2_ops = {
.set_bus_width = sdhci_set_bus_width,
.reset = sdhci_at91_reset,
.set_uhs_signaling = sdhci_at91_set_uhs_signaling,
- .set_power = sdhci_at91_set_power,
+ .set_power = sdhci_set_power_and_bus_voltage,
};
static const struct sdhci_pltfm_data sdhci_sama5d2_pdata = {
--
2.25.1
The sdhci core provides a helper function with the same functionality as
this controller's set_power() callback. Use it instead.
Signed-off-by: Nicolas Saenz Julienne <[email protected]>
---
drivers/mmc/host/sdhci-of-arasan.c | 15 ++-------------
1 file changed, 2 insertions(+), 13 deletions(-)
diff --git a/drivers/mmc/host/sdhci-of-arasan.c b/drivers/mmc/host/sdhci-of-arasan.c
index 0146d7dd315b..d4905c106c06 100644
--- a/drivers/mmc/host/sdhci-of-arasan.c
+++ b/drivers/mmc/host/sdhci-of-arasan.c
@@ -325,17 +325,6 @@ static int sdhci_arasan_voltage_switch(struct mmc_host *mmc,
return -EINVAL;
}
-static void sdhci_arasan_set_power(struct sdhci_host *host, unsigned char mode,
- unsigned short vdd)
-{
- if (!IS_ERR(host->mmc->supply.vmmc)) {
- struct mmc_host *mmc = host->mmc;
-
- mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd);
- }
- sdhci_set_power_noreg(host, mode, vdd);
-}
-
static const struct sdhci_ops sdhci_arasan_ops = {
.set_clock = sdhci_arasan_set_clock,
.get_max_clock = sdhci_pltfm_clk_get_max_clock,
@@ -343,7 +332,7 @@ static const struct sdhci_ops sdhci_arasan_ops = {
.set_bus_width = sdhci_set_bus_width,
.reset = sdhci_arasan_reset,
.set_uhs_signaling = sdhci_set_uhs_signaling,
- .set_power = sdhci_arasan_set_power,
+ .set_power = sdhci_set_power_and_bus_voltage,
};
static const struct sdhci_pltfm_data sdhci_arasan_pdata = {
@@ -414,7 +403,7 @@ static const struct sdhci_ops sdhci_arasan_cqe_ops = {
.set_bus_width = sdhci_set_bus_width,
.reset = sdhci_arasan_reset,
.set_uhs_signaling = sdhci_set_uhs_signaling,
- .set_power = sdhci_arasan_set_power,
+ .set_power = sdhci_set_power_and_bus_voltage,
.irq = sdhci_arasan_cqhci_irq,
};
--
2.25.1
Some controllers diverge from the standard way of setting power and need
their bus voltage register to be configured regardless of the whether
they use regulators. As this is a common pattern across sdhci hosts,
create a helper function.
Signed-off-by: Nicolas Saenz Julienne <[email protected]>
---
drivers/mmc/host/sdhci.c | 19 +++++++++++++++++++
drivers/mmc/host/sdhci.h | 3 +++
2 files changed, 22 insertions(+)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 9c3745118e49..6ed11f9554e8 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2010,6 +2010,25 @@ void sdhci_set_power(struct sdhci_host *host, unsigned char mode,
}
EXPORT_SYMBOL_GPL(sdhci_set_power);
+/*
+ * Some controllers need to configure a valid bus voltage on their power
+ * register regardless of whether an external regulator is taking care of power
+ * supply. This helper function takes care of it if set as the controller's
+ * sdhci_ops.set_power callback.
+ */
+void sdhci_set_power_and_bus_voltage(struct sdhci_host *host,
+ unsigned char mode,
+ unsigned short vdd)
+{
+ if (!IS_ERR(host->mmc->supply.vmmc)) {
+ struct mmc_host *mmc = host->mmc;
+
+ mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd);
+ }
+ sdhci_set_power_noreg(host, mode, vdd);
+}
+EXPORT_SYMBOL_GPL(sdhci_set_power_and_bus_voltage);
+
/*****************************************************************************\
* *
* MMC callbacks *
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index cac2d97782e6..1be7c18264cd 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -772,6 +772,9 @@ void sdhci_set_clock(struct sdhci_host *host, unsigned int clock);
void sdhci_enable_clk(struct sdhci_host *host, u16 clk);
void sdhci_set_power(struct sdhci_host *host, unsigned char mode,
unsigned short vdd);
+void sdhci_set_power_and_bus_voltage(struct sdhci_host *host,
+ unsigned char mode,
+ unsigned short vdd);
void sdhci_set_power_noreg(struct sdhci_host *host, unsigned char mode,
unsigned short vdd);
void sdhci_request(struct mmc_host *mmc, struct mmc_request *mrq);
--
2.25.1
The controller needs a valid bus voltage in its power register
regardless of whether an external regulator is taking care of the power
supply.
The sdhci core already provides a helper function for this,
sdhci_set_power_and_bus_voltage(), so create a bcm2711 specific 'struct
sdhci_ops' which makes use of it.
Signed-off-by: Nicolas Saenz Julienne <[email protected]>
---
drivers/mmc/host/sdhci-iproc.c | 17 ++++++++++++++++-
1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/drivers/mmc/host/sdhci-iproc.c b/drivers/mmc/host/sdhci-iproc.c
index f4f5f0a70cda..225603148d7d 100644
--- a/drivers/mmc/host/sdhci-iproc.c
+++ b/drivers/mmc/host/sdhci-iproc.c
@@ -261,9 +261,24 @@ static const struct sdhci_iproc_data bcm2835_data = {
.mmc_caps = 0x00000000,
};
+static const struct sdhci_ops sdhci_iproc_bcm2711_ops = {
+ .read_l = sdhci_iproc_readl,
+ .read_w = sdhci_iproc_readw,
+ .read_b = sdhci_iproc_readb,
+ .write_l = sdhci_iproc_writel,
+ .write_w = sdhci_iproc_writew,
+ .write_b = sdhci_iproc_writeb,
+ .set_clock = sdhci_set_clock,
+ .set_power = sdhci_set_power_and_bus_voltage,
+ .get_max_clock = sdhci_iproc_get_max_clock,
+ .set_bus_width = sdhci_set_bus_width,
+ .reset = sdhci_reset,
+ .set_uhs_signaling = sdhci_set_uhs_signaling,
+};
+
static const struct sdhci_pltfm_data sdhci_bcm2711_pltfm_data = {
.quirks = SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12,
- .ops = &sdhci_iproc_32only_ops,
+ .ops = &sdhci_iproc_bcm2711_ops,
};
static const struct sdhci_iproc_data bcm2711_data = {
--
2.25.1
The sdhci core provides a helper function with the same functionality as
this controller's set_power() callback. Use it instead.
Signed-off-by: Nicolas Saenz Julienne <[email protected]>
---
drivers/mmc/host/sdhci-pxav3.c | 20 +-------------------
1 file changed, 1 insertion(+), 19 deletions(-)
diff --git a/drivers/mmc/host/sdhci-pxav3.c b/drivers/mmc/host/sdhci-pxav3.c
index e55037ceda73..75fe90b88f9b 100644
--- a/drivers/mmc/host/sdhci-pxav3.c
+++ b/drivers/mmc/host/sdhci-pxav3.c
@@ -297,27 +297,9 @@ static void pxav3_set_uhs_signaling(struct sdhci_host *host, unsigned int uhs)
__func__, uhs, ctrl_2);
}
-static void pxav3_set_power(struct sdhci_host *host, unsigned char mode,
- unsigned short vdd)
-{
- struct mmc_host *mmc = host->mmc;
- u8 pwr = host->pwr;
-
- sdhci_set_power_noreg(host, mode, vdd);
-
- if (host->pwr == pwr)
- return;
-
- if (host->pwr == 0)
- vdd = 0;
-
- if (!IS_ERR(mmc->supply.vmmc))
- mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd);
-}
-
static const struct sdhci_ops pxav3_sdhci_ops = {
.set_clock = sdhci_set_clock,
- .set_power = pxav3_set_power,
+ .set_power = sdhci_set_power_and_bus_voltage,
.platform_send_init_74_clocks = pxav3_gen_init_74_clocks,
.get_max_clock = sdhci_pltfm_clk_get_max_clock,
.set_bus_width = sdhci_set_bus_width,
--
2.25.1
On 6/03/20 7:44 pm, Nicolas Saenz Julienne wrote:
> The sdhci core provides a helper function with the same functionality as
> this controller's set_power() callback. Use it instead.
>
> Signed-off-by: Nicolas Saenz Julienne <[email protected]>
> ---
> drivers/mmc/host/sdhci-pxav3.c | 20 +-------------------
> 1 file changed, 1 insertion(+), 19 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-pxav3.c b/drivers/mmc/host/sdhci-pxav3.c
> index e55037ceda73..75fe90b88f9b 100644
> --- a/drivers/mmc/host/sdhci-pxav3.c
> +++ b/drivers/mmc/host/sdhci-pxav3.c
> @@ -297,27 +297,9 @@ static void pxav3_set_uhs_signaling(struct sdhci_host *host, unsigned int uhs)
> __func__, uhs, ctrl_2);
> }
>
> -static void pxav3_set_power(struct sdhci_host *host, unsigned char mode,
> - unsigned short vdd)
> -{
> - struct mmc_host *mmc = host->mmc;
> - u8 pwr = host->pwr;
> -
> - sdhci_set_power_noreg(host, mode, vdd);
> -
> - if (host->pwr == pwr)
> - return;
> -
> - if (host->pwr == 0)
> - vdd = 0;
> -
> - if (!IS_ERR(mmc->supply.vmmc))
> - mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd);
> -}
This code is different. The commit message should explain why it is
equivalent. Has it been tested?
> -
> static const struct sdhci_ops pxav3_sdhci_ops = {
> .set_clock = sdhci_set_clock,
> - .set_power = pxav3_set_power,
> + .set_power = sdhci_set_power_and_bus_voltage,
> .platform_send_init_74_clocks = pxav3_gen_init_74_clocks,
> .get_max_clock = sdhci_pltfm_clk_get_max_clock,
> .set_bus_width = sdhci_set_bus_width,
>
On 6/03/20 7:44 pm, Nicolas Saenz Julienne wrote:
> Some controllers diverge from the standard way of setting power and need
> their bus voltage register to be configured regardless of the whether
> they use regulators. As this is a common pattern across sdhci hosts,
> create a helper function.
>
> Signed-off-by: Nicolas Saenz Julienne <[email protected]>
Acked-by: Adrian Hunter <[email protected]>
> ---
> drivers/mmc/host/sdhci.c | 19 +++++++++++++++++++
> drivers/mmc/host/sdhci.h | 3 +++
> 2 files changed, 22 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index 9c3745118e49..6ed11f9554e8 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -2010,6 +2010,25 @@ void sdhci_set_power(struct sdhci_host *host, unsigned char mode,
> }
> EXPORT_SYMBOL_GPL(sdhci_set_power);
>
> +/*
> + * Some controllers need to configure a valid bus voltage on their power
> + * register regardless of whether an external regulator is taking care of power
> + * supply. This helper function takes care of it if set as the controller's
> + * sdhci_ops.set_power callback.
> + */
> +void sdhci_set_power_and_bus_voltage(struct sdhci_host *host,
> + unsigned char mode,
> + unsigned short vdd)
> +{
> + if (!IS_ERR(host->mmc->supply.vmmc)) {
> + struct mmc_host *mmc = host->mmc;
> +
> + mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd);
> + }
> + sdhci_set_power_noreg(host, mode, vdd);
> +}
> +EXPORT_SYMBOL_GPL(sdhci_set_power_and_bus_voltage);
> +
> /*****************************************************************************\
> * *
> * MMC callbacks *
> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
> index cac2d97782e6..1be7c18264cd 100644
> --- a/drivers/mmc/host/sdhci.h
> +++ b/drivers/mmc/host/sdhci.h
> @@ -772,6 +772,9 @@ void sdhci_set_clock(struct sdhci_host *host, unsigned int clock);
> void sdhci_enable_clk(struct sdhci_host *host, u16 clk);
> void sdhci_set_power(struct sdhci_host *host, unsigned char mode,
> unsigned short vdd);
> +void sdhci_set_power_and_bus_voltage(struct sdhci_host *host,
> + unsigned char mode,
> + unsigned short vdd);
> void sdhci_set_power_noreg(struct sdhci_host *host, unsigned char mode,
> unsigned short vdd);
> void sdhci_request(struct mmc_host *mmc, struct mmc_request *mrq);
>
On 6/03/20 7:44 pm, Nicolas Saenz Julienne wrote:
> The sdhci core provides a helper function with the same functionality as
> this controller's set_power() callback. Use it instead.
>
> Signed-off-by: Nicolas Saenz Julienne <[email protected]>
> ---
> drivers/mmc/host/sdhci-xenon.c | 20 +-------------------
> 1 file changed, 1 insertion(+), 19 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-xenon.c b/drivers/mmc/host/sdhci-xenon.c
> index 1dea1ba66f7b..1e9a7a76f2ba 100644
> --- a/drivers/mmc/host/sdhci-xenon.c
> +++ b/drivers/mmc/host/sdhci-xenon.c
> @@ -213,24 +213,6 @@ static void xenon_set_uhs_signaling(struct sdhci_host *host,
> sdhci_writew(host, ctrl_2, SDHCI_HOST_CONTROL2);
> }
>
> -static void xenon_set_power(struct sdhci_host *host, unsigned char mode,
> - unsigned short vdd)
> -{
> - struct mmc_host *mmc = host->mmc;
> - u8 pwr = host->pwr;
> -
> - sdhci_set_power_noreg(host, mode, vdd);
> -
> - if (host->pwr == pwr)
> - return;
> -
> - if (host->pwr == 0)
> - vdd = 0;
> -
> - if (!IS_ERR(mmc->supply.vmmc))
> - mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd);
> -}
This code is different. The commit message should explain why it is
equivalent. Has it been tested?
> -
> static void xenon_voltage_switch(struct sdhci_host *host)
> {
> /* Wait for 5ms after set 1.8V signal enable bit */
> @@ -240,7 +222,7 @@ static void xenon_voltage_switch(struct sdhci_host *host)
> static const struct sdhci_ops sdhci_xenon_ops = {
> .voltage_switch = xenon_voltage_switch,
> .set_clock = sdhci_set_clock,
> - .set_power = xenon_set_power,
> + .set_power = sdhci_set_power_and_bus_voltage,
> .set_bus_width = sdhci_set_bus_width,
> .reset = xenon_reset,
> .set_uhs_signaling = xenon_set_uhs_signaling,
>
On 6/03/20 7:44 pm, Nicolas Saenz Julienne wrote:
> The sdhci core provides a helper function with the same functionality as
> this controller's set_power() callback. Use it instead.
>
> Signed-off-by: Nicolas Saenz Julienne <[email protected]>
Acked-by: Adrian Hunter <[email protected]>
> ---
> drivers/mmc/host/sdhci-of-arasan.c | 15 ++-------------
> 1 file changed, 2 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-of-arasan.c b/drivers/mmc/host/sdhci-of-arasan.c
> index 0146d7dd315b..d4905c106c06 100644
> --- a/drivers/mmc/host/sdhci-of-arasan.c
> +++ b/drivers/mmc/host/sdhci-of-arasan.c
> @@ -325,17 +325,6 @@ static int sdhci_arasan_voltage_switch(struct mmc_host *mmc,
> return -EINVAL;
> }
>
> -static void sdhci_arasan_set_power(struct sdhci_host *host, unsigned char mode,
> - unsigned short vdd)
> -{
> - if (!IS_ERR(host->mmc->supply.vmmc)) {
> - struct mmc_host *mmc = host->mmc;
> -
> - mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd);
> - }
> - sdhci_set_power_noreg(host, mode, vdd);
> -}
> -
> static const struct sdhci_ops sdhci_arasan_ops = {
> .set_clock = sdhci_arasan_set_clock,
> .get_max_clock = sdhci_pltfm_clk_get_max_clock,
> @@ -343,7 +332,7 @@ static const struct sdhci_ops sdhci_arasan_ops = {
> .set_bus_width = sdhci_set_bus_width,
> .reset = sdhci_arasan_reset,
> .set_uhs_signaling = sdhci_set_uhs_signaling,
> - .set_power = sdhci_arasan_set_power,
> + .set_power = sdhci_set_power_and_bus_voltage,
> };
>
> static const struct sdhci_pltfm_data sdhci_arasan_pdata = {
> @@ -414,7 +403,7 @@ static const struct sdhci_ops sdhci_arasan_cqe_ops = {
> .set_bus_width = sdhci_set_bus_width,
> .reset = sdhci_arasan_reset,
> .set_uhs_signaling = sdhci_set_uhs_signaling,
> - .set_power = sdhci_arasan_set_power,
> + .set_power = sdhci_set_power_and_bus_voltage,
> .irq = sdhci_arasan_cqhci_irq,
> };
>
>
On 6/03/20 7:44 pm, Nicolas Saenz Julienne wrote:
> The sdhci core provides a helper function with the same functionality as
> this controller's set_power() callback. Use it instead.
>
> Signed-off-by: Nicolas Saenz Julienne <[email protected]>
Acked-by: Adrian Hunter <[email protected]>
> ---
> drivers/mmc/host/sdhci-milbeaut.c | 13 +------------
> 1 file changed, 1 insertion(+), 12 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-milbeaut.c b/drivers/mmc/host/sdhci-milbeaut.c
> index 92f30a1db435..4e7cc0680f94 100644
> --- a/drivers/mmc/host/sdhci-milbeaut.c
> +++ b/drivers/mmc/host/sdhci-milbeaut.c
> @@ -121,17 +121,6 @@ static void sdhci_milbeaut_reset(struct sdhci_host *host, u8 mask)
> }
> }
>
> -static void sdhci_milbeaut_set_power(struct sdhci_host *host,
> - unsigned char mode, unsigned short vdd)
> -{
> - if (!IS_ERR(host->mmc->supply.vmmc)) {
> - struct mmc_host *mmc = host->mmc;
> -
> - mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd);
> - }
> - sdhci_set_power_noreg(host, mode, vdd);
> -}
> -
> static const struct sdhci_ops sdhci_milbeaut_ops = {
> .voltage_switch = sdhci_milbeaut_soft_voltage_switch,
> .get_min_clock = sdhci_milbeaut_get_min_clock,
> @@ -139,7 +128,7 @@ static const struct sdhci_ops sdhci_milbeaut_ops = {
> .set_clock = sdhci_set_clock,
> .set_bus_width = sdhci_set_bus_width,
> .set_uhs_signaling = sdhci_set_uhs_signaling,
> - .set_power = sdhci_milbeaut_set_power,
> + .set_power = sdhci_set_power_and_bus_voltage,
> };
>
> static void sdhci_milbeaut_bridge_reset(struct sdhci_host *host,
>
On 6/03/20 7:44 pm, Nicolas Saenz Julienne wrote:
> The sdhci core provides a helper function with the same functionality as
> this controller's set_power() callback. Use it instead.
>
> Signed-off-by: Nicolas Saenz Julienne <[email protected]>
Acked-by: Adrian Hunter <[email protected]>
> ---
> drivers/mmc/host/sdhci-of-at91.c | 18 +-----------------
> 1 file changed, 1 insertion(+), 17 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-of-at91.c b/drivers/mmc/host/sdhci-of-at91.c
> index ab2bd314a390..564a606f6729 100644
> --- a/drivers/mmc/host/sdhci-of-at91.c
> +++ b/drivers/mmc/host/sdhci-of-at91.c
> @@ -101,22 +101,6 @@ static void sdhci_at91_set_clock(struct sdhci_host *host, unsigned int clock)
> sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
> }
>
> -/*
> - * In this specific implementation of the SDHCI controller, the power register
> - * needs to have a valid voltage set even when the power supply is managed by
> - * an external regulator.
> - */
> -static void sdhci_at91_set_power(struct sdhci_host *host, unsigned char mode,
> - unsigned short vdd)
> -{
> - if (!IS_ERR(host->mmc->supply.vmmc)) {
> - struct mmc_host *mmc = host->mmc;
> -
> - mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd);
> - }
> - sdhci_set_power_noreg(host, mode, vdd);
> -}
> -
> static void sdhci_at91_set_uhs_signaling(struct sdhci_host *host,
> unsigned int timing)
> {
> @@ -145,7 +129,7 @@ static const struct sdhci_ops sdhci_at91_sama5d2_ops = {
> .set_bus_width = sdhci_set_bus_width,
> .reset = sdhci_at91_reset,
> .set_uhs_signaling = sdhci_at91_set_uhs_signaling,
> - .set_power = sdhci_at91_set_power,
> + .set_power = sdhci_set_power_and_bus_voltage,
> };
>
> static const struct sdhci_pltfm_data sdhci_sama5d2_pdata = {
>
On 6/03/20 7:44 pm, Nicolas Saenz Julienne wrote:
> The sdhci core provides a helper function with the same functionality as
> this controller's set_power() callback. Use it instead.
>
> Signed-off-by: Nicolas Saenz Julienne <[email protected]>
Acked-by: Adrian Hunter <[email protected]>
> ---
> drivers/mmc/host/sdhci_am654.c | 17 +++--------------
> 1 file changed, 3 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci_am654.c b/drivers/mmc/host/sdhci_am654.c
> index 3afea580fbea..c70647489bbd 100644
> --- a/drivers/mmc/host/sdhci_am654.c
> +++ b/drivers/mmc/host/sdhci_am654.c
> @@ -208,17 +208,6 @@ static void sdhci_j721e_4bit_set_clock(struct sdhci_host *host,
> sdhci_set_clock(host, clock);
> }
>
> -static void sdhci_am654_set_power(struct sdhci_host *host, unsigned char mode,
> - unsigned short vdd)
> -{
> - if (!IS_ERR(host->mmc->supply.vmmc)) {
> - struct mmc_host *mmc = host->mmc;
> -
> - mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd);
> - }
> - sdhci_set_power_noreg(host, mode, vdd);
> -}
> -
> static void sdhci_am654_write_b(struct sdhci_host *host, u8 val, int reg)
> {
> unsigned char timing = host->mmc->ios.timing;
> @@ -274,7 +263,7 @@ static struct sdhci_ops sdhci_am654_ops = {
> .get_timeout_clock = sdhci_pltfm_clk_get_max_clock,
> .set_uhs_signaling = sdhci_set_uhs_signaling,
> .set_bus_width = sdhci_set_bus_width,
> - .set_power = sdhci_am654_set_power,
> + .set_power = sdhci_set_power_and_bus_voltage,
> .set_clock = sdhci_am654_set_clock,
> .write_b = sdhci_am654_write_b,
> .irq = sdhci_am654_cqhci_irq,
> @@ -297,7 +286,7 @@ static struct sdhci_ops sdhci_j721e_8bit_ops = {
> .get_timeout_clock = sdhci_pltfm_clk_get_max_clock,
> .set_uhs_signaling = sdhci_set_uhs_signaling,
> .set_bus_width = sdhci_set_bus_width,
> - .set_power = sdhci_am654_set_power,
> + .set_power = sdhci_set_power_and_bus_voltage,
> .set_clock = sdhci_am654_set_clock,
> .write_b = sdhci_am654_write_b,
> .irq = sdhci_am654_cqhci_irq,
> @@ -320,7 +309,7 @@ static struct sdhci_ops sdhci_j721e_4bit_ops = {
> .get_timeout_clock = sdhci_pltfm_clk_get_max_clock,
> .set_uhs_signaling = sdhci_set_uhs_signaling,
> .set_bus_width = sdhci_set_bus_width,
> - .set_power = sdhci_am654_set_power,
> + .set_power = sdhci_set_power_and_bus_voltage,
> .set_clock = sdhci_j721e_4bit_set_clock,
> .write_b = sdhci_am654_write_b,
> .irq = sdhci_am654_cqhci_irq,
>
On 6/03/20 7:44 pm, Nicolas Saenz Julienne wrote:
> The controller needs a valid bus voltage in its power register
> regardless of whether an external regulator is taking care of the power
> supply.
>
> The sdhci core already provides a helper function for this,
> sdhci_set_power_and_bus_voltage(), so create a bcm2711 specific 'struct
> sdhci_ops' which makes use of it.
>
> Signed-off-by: Nicolas Saenz Julienne <[email protected]>
Acked-by: Adrian Hunter <[email protected]>
> ---
> drivers/mmc/host/sdhci-iproc.c | 17 ++++++++++++++++-
> 1 file changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/sdhci-iproc.c b/drivers/mmc/host/sdhci-iproc.c
> index f4f5f0a70cda..225603148d7d 100644
> --- a/drivers/mmc/host/sdhci-iproc.c
> +++ b/drivers/mmc/host/sdhci-iproc.c
> @@ -261,9 +261,24 @@ static const struct sdhci_iproc_data bcm2835_data = {
> .mmc_caps = 0x00000000,
> };
>
> +static const struct sdhci_ops sdhci_iproc_bcm2711_ops = {
> + .read_l = sdhci_iproc_readl,
> + .read_w = sdhci_iproc_readw,
> + .read_b = sdhci_iproc_readb,
> + .write_l = sdhci_iproc_writel,
> + .write_w = sdhci_iproc_writew,
> + .write_b = sdhci_iproc_writeb,
> + .set_clock = sdhci_set_clock,
> + .set_power = sdhci_set_power_and_bus_voltage,
> + .get_max_clock = sdhci_iproc_get_max_clock,
> + .set_bus_width = sdhci_set_bus_width,
> + .reset = sdhci_reset,
> + .set_uhs_signaling = sdhci_set_uhs_signaling,
> +};
> +
> static const struct sdhci_pltfm_data sdhci_bcm2711_pltfm_data = {
> .quirks = SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12,
> - .ops = &sdhci_iproc_32only_ops,
> + .ops = &sdhci_iproc_bcm2711_ops,
> };
>
> static const struct sdhci_iproc_data bcm2711_data = {
>
On Mon, 2020-03-09 at 09:20 +0200, Adrian Hunter wrote:
> > -static void xenon_set_power(struct sdhci_host *host, unsigned char mode,
> > - unsigned short vdd)
> > -{
> > - struct mmc_host *mmc = host->mmc;
> > - u8 pwr = host->pwr;
> > -
> > - sdhci_set_power_noreg(host, mode, vdd);
> > -
> > - if (host->pwr == pwr)
> > - return;
> > -
> > - if (host->pwr == 0)
> > - vdd = 0;
> > -
> > - if (!IS_ERR(mmc->supply.vmmc))
> > - mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd);
> > -}
>
> This code is different. The commit message should explain why it is
> equivalent. Has it been tested?
Yes, I should've pointed it out. The rationale behind including sdhci-xenon and
sdhci-pxav3 is based on xenon's original commit message (99c14fc360dbb):
mmc: sdhci-xenon: add set_power callback
Xenon sdh controller requests proper SD bus voltage select
bits programmed even with vmmc power supply. Any reserved
value(100b-000b) programmed in this field will lead to controller
ignore SD bus power bit and keep its value at zero.
Add set_power callback to handle this.
I can't test it, but it felt to me as the implementation differences are only
there as different people wrote the code. Ultimately, I'll be happy to drop
them from the series if you feel it's too much of an assumption, I can see how
the controllers could react badly to the ordering change. If not I can send a
v3 with fixed commit messages.
Regards,
Nicolas
On 9/03/20 12:53 pm, Nicolas Saenz Julienne wrote:
> On Mon, 2020-03-09 at 09:20 +0200, Adrian Hunter wrote:
>>> -static void xenon_set_power(struct sdhci_host *host, unsigned char mode,
>>> - unsigned short vdd)
>>> -{
>>> - struct mmc_host *mmc = host->mmc;
>>> - u8 pwr = host->pwr;
>>> -
>>> - sdhci_set_power_noreg(host, mode, vdd);
>>> -
>>> - if (host->pwr == pwr)
>>> - return;
>>> -
>>> - if (host->pwr == 0)
>>> - vdd = 0;
>>> -
>>> - if (!IS_ERR(mmc->supply.vmmc))
>>> - mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd);
>>> -}
>>
>> This code is different. The commit message should explain why it is
>> equivalent. Has it been tested?
>
> Yes, I should've pointed it out. The rationale behind including sdhci-xenon and
> sdhci-pxav3 is based on xenon's original commit message (99c14fc360dbb):
>
> mmc: sdhci-xenon: add set_power callback
>
> Xenon sdh controller requests proper SD bus voltage select
> bits programmed even with vmmc power supply. Any reserved
> value(100b-000b) programmed in this field will lead to controller
> ignore SD bus power bit and keep its value at zero.
> Add set_power callback to handle this.
>
> I can't test it, but it felt to me as the implementation differences are only
> there as different people wrote the code. Ultimately, I'll be happy to drop
> them from the series if you feel it's too much of an assumption, I can see how
> the controllers could react badly to the ordering change. If not I can send a
> v3 with fixed commit messages.
We can wait a bit and see if anyone provides a Tested-by tag, otherwise
safer to drop it.
+ Zhoujie Wu <[email protected]>
On 9/03/20 1:55 pm, Adrian Hunter wrote:
> On 9/03/20 12:53 pm, Nicolas Saenz Julienne wrote:
>> On Mon, 2020-03-09 at 09:20 +0200, Adrian Hunter wrote:
>>>> -static void xenon_set_power(struct sdhci_host *host, unsigned char mode,
>>>> - unsigned short vdd)
>>>> -{
>>>> - struct mmc_host *mmc = host->mmc;
>>>> - u8 pwr = host->pwr;
>>>> -
>>>> - sdhci_set_power_noreg(host, mode, vdd);
>>>> -
>>>> - if (host->pwr == pwr)
>>>> - return;
>>>> -
>>>> - if (host->pwr == 0)
>>>> - vdd = 0;
>>>> -
>>>> - if (!IS_ERR(mmc->supply.vmmc))
>>>> - mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd);
>>>> -}
>>>
>>> This code is different. The commit message should explain why it is
>>> equivalent. Has it been tested?
>>
>> Yes, I should've pointed it out. The rationale behind including sdhci-xenon and
>> sdhci-pxav3 is based on xenon's original commit message (99c14fc360dbb):
>>
>> mmc: sdhci-xenon: add set_power callback
>>
>> Xenon sdh controller requests proper SD bus voltage select
>> bits programmed even with vmmc power supply. Any reserved
>> value(100b-000b) programmed in this field will lead to controller
>> ignore SD bus power bit and keep its value at zero.
>> Add set_power callback to handle this.
>>
>> I can't test it, but it felt to me as the implementation differences are only
>> there as different people wrote the code. Ultimately, I'll be happy to drop
>> them from the series if you feel it's too much of an assumption, I can see how
>> the controllers could react badly to the ordering change. If not I can send a
>> v3 with fixed commit messages.
>
> We can wait a bit and see if anyone provides a Tested-by tag, otherwise
> safer to drop it.
>
Hi Nicolas,
Am 06.03.20 um 18:44 schrieb Nicolas Saenz Julienne:
> The SD card power can be controlled trough a pin routed into the board's
> external GPIO expander. Turn that into a regulator and provide it to
> emmc2.
>
> Signed-off-by: Nicolas Saenz Julienne <[email protected]>
> ---
> arch/arm/boot/dts/bcm2711-rpi-4-b.dts | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
> index b0ea8233b636..a2da058396fe 100644
> --- a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
> +++ b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
> @@ -55,6 +55,16 @@ sd_io_1v8_reg: sd_io_1v8_reg {
> 3300000 0x0>;
> status = "okay";
> };
> +
> + sd_vcc_reg: sd_vcc_reg {
> + compatible = "regulator-fixed";
i think we need to enable CONFIG_REGULATOR_FIXED_VOLTAGE in
bcm2835_defconfig
Best regards
Stefan
> + regulator-name = "vcc-sd";
> + regulator-min-microvolt = <3300000>;
> + regulator-max-microvolt = <3300000>;
> + regulator-boot-on;
> + enable-active-high;
> + gpio = <&expgpio 6 GPIO_ACTIVE_HIGH>;
> + };
> };
>
> &firmware {
> @@ -173,6 +183,7 @@ brcmf: wifi@1 {
> /* EMMC2 is used to drive the SD card */
> &emmc2 {
> vqmmc-supply = <&sd_io_1v8_reg>;
> + vmmc-supply = <&sd_vcc_reg>;
> broken-cd;
> status = "okay";
> };
On Mon, 2020-03-09 at 21:00 +0100, Stefan Wahren wrote:
> Hi Nicolas,
>
> Am 06.03.20 um 18:44 schrieb Nicolas Saenz Julienne:
> > The SD card power can be controlled trough a pin routed into the board's
> > external GPIO expander. Turn that into a regulator and provide it to
> > emmc2.
> >
> > Signed-off-by: Nicolas Saenz Julienne <[email protected]>
> > ---
> > arch/arm/boot/dts/bcm2711-rpi-4-b.dts | 11 +++++++++++
> > 1 file changed, 11 insertions(+)
> >
> > diff --git a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
> > b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
> > index b0ea8233b636..a2da058396fe 100644
> > --- a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
> > +++ b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
> > @@ -55,6 +55,16 @@ sd_io_1v8_reg: sd_io_1v8_reg {
> > 3300000 0x0>;
> > status = "okay";
> > };
> > +
> > + sd_vcc_reg: sd_vcc_reg {
> > + compatible = "regulator-fixed";
>
> i think we need to enable CONFIG_REGULATOR_FIXED_VOLTAGE in
> bcm2835_defconfig
I'll take it into account for v3,
Thanks!
Nicolas
On Fri, 6 Mar 2020 at 18:44, Nicolas Saenz Julienne
<[email protected]> wrote:
>
> The series first cleans up a common pattern, which is ultimately needed
> to integrate the regulator with bcm2711's sdhci-iproc. It then
> introduces the relevant device-tree changes.
>
> ---
>
> Changes since v1:
> - Use helper function istead of quirk
> - Add GPIO label
>
> Nicolas Saenz Julienne (11):
> mmc: sdhci: Introduce sdhci_set_power_and_bus_voltage()
> mmc: sdhci: arasan: Use sdhci_set_power_and_voltage()
> mmc: sdhci: milbeaut: Use sdhci_set_power_and_voltage()
> mmc: sdhci: at91: Use sdhci_set_power_and_voltage()
> mmc: sdhci: pxav3: Use sdhci_set_power_and_voltage()
> mmc: sdhci: xenon: Use sdhci_set_power_and_voltage()
> mmc: sdhci: am654: Use sdhci_set_power_and_voltage()
> mmc: sdhci: Unexport sdhci_set_power_noreg()
> mmc: sdhci: iproc: Add custom set_power() callback for bcm2711
> ARM: dts: bcm2711: Update expgpio's GPIO labels
> ARM: dts: bcm2711: Add vmmc regulator in emmc2
>
> arch/arm/boot/dts/bcm2711-rpi-4-b.dts | 13 ++++++++++++-
> drivers/mmc/host/sdhci-iproc.c | 17 ++++++++++++++++-
> drivers/mmc/host/sdhci-milbeaut.c | 13 +------------
> drivers/mmc/host/sdhci-of-arasan.c | 15 ++-------------
> drivers/mmc/host/sdhci-of-at91.c | 18 +-----------------
> drivers/mmc/host/sdhci-pxav3.c | 20 +-------------------
> drivers/mmc/host/sdhci-xenon.c | 20 +-------------------
> drivers/mmc/host/sdhci.c | 24 +++++++++++++++++++++---
> drivers/mmc/host/sdhci.h | 5 +++--
> drivers/mmc/host/sdhci_am654.c | 17 +++--------------
> 10 files changed, 61 insertions(+), 101 deletions(-)
>
> --
> 2.25.1
>
Patch 1-4, 6, 9 applied for next, thanks!
Kind regards
Uffe
Hi Ulf,
On Thu, 2020-03-12 at 14:08 +0100, Ulf Hansson wrote:
> On Fri, 6 Mar 2020 at 18:44, Nicolas Saenz Julienne
> <[email protected]> wrote:
> > The series first cleans up a common pattern, which is ultimately needed
> > to integrate the regulator with bcm2711's sdhci-iproc. It then
> > introduces the relevant device-tree changes.
> >
> > ---
> >
> > Changes since v1:
> > - Use helper function istead of quirk
> > - Add GPIO label
> >
> > Nicolas Saenz Julienne (11):
> > mmc: sdhci: Introduce sdhci_set_power_and_bus_voltage()
> > mmc: sdhci: arasan: Use sdhci_set_power_and_voltage()
> > mmc: sdhci: milbeaut: Use sdhci_set_power_and_voltage()
> > mmc: sdhci: at91: Use sdhci_set_power_and_voltage()
> > mmc: sdhci: pxav3: Use sdhci_set_power_and_voltage()
> > mmc: sdhci: xenon: Use sdhci_set_power_and_voltage()
> > mmc: sdhci: am654: Use sdhci_set_power_and_voltage()
> > mmc: sdhci: Unexport sdhci_set_power_noreg()
> > mmc: sdhci: iproc: Add custom set_power() callback for bcm2711
> > ARM: dts: bcm2711: Update expgpio's GPIO labels
> > ARM: dts: bcm2711: Add vmmc regulator in emmc2
> >
> > arch/arm/boot/dts/bcm2711-rpi-4-b.dts | 13 ++++++++++++-
> > drivers/mmc/host/sdhci-iproc.c | 17 ++++++++++++++++-
> > drivers/mmc/host/sdhci-milbeaut.c | 13 +------------
> > drivers/mmc/host/sdhci-of-arasan.c | 15 ++-------------
> > drivers/mmc/host/sdhci-of-at91.c | 18 +-----------------
> > drivers/mmc/host/sdhci-pxav3.c | 20 +-------------------
> > drivers/mmc/host/sdhci-xenon.c | 20 +-------------------
> > drivers/mmc/host/sdhci.c | 24 +++++++++++++++++++++---
> > drivers/mmc/host/sdhci.h | 5 +++--
> > drivers/mmc/host/sdhci_am654.c | 17 +++--------------
> > 10 files changed, 61 insertions(+), 101 deletions(-)
> >
> > --
> > 2.25.1
> >
>
> Patch 1-4, 6, 9 applied for next, thanks!
I think you meant to apply 1-4, 7 and 9. Patch 6 is one of the contentious
ones.
Regards,
Nicolas
On Thu, 12 Mar 2020 at 14:13, Nicolas Saenz Julienne
<[email protected]> wrote:
>
> Hi Ulf,
>
> On Thu, 2020-03-12 at 14:08 +0100, Ulf Hansson wrote:
> > On Fri, 6 Mar 2020 at 18:44, Nicolas Saenz Julienne
> > <[email protected]> wrote:
> > > The series first cleans up a common pattern, which is ultimately needed
> > > to integrate the regulator with bcm2711's sdhci-iproc. It then
> > > introduces the relevant device-tree changes.
> > >
> > > ---
> > >
> > > Changes since v1:
> > > - Use helper function istead of quirk
> > > - Add GPIO label
> > >
> > > Nicolas Saenz Julienne (11):
> > > mmc: sdhci: Introduce sdhci_set_power_and_bus_voltage()
> > > mmc: sdhci: arasan: Use sdhci_set_power_and_voltage()
> > > mmc: sdhci: milbeaut: Use sdhci_set_power_and_voltage()
> > > mmc: sdhci: at91: Use sdhci_set_power_and_voltage()
> > > mmc: sdhci: pxav3: Use sdhci_set_power_and_voltage()
> > > mmc: sdhci: xenon: Use sdhci_set_power_and_voltage()
> > > mmc: sdhci: am654: Use sdhci_set_power_and_voltage()
> > > mmc: sdhci: Unexport sdhci_set_power_noreg()
> > > mmc: sdhci: iproc: Add custom set_power() callback for bcm2711
> > > ARM: dts: bcm2711: Update expgpio's GPIO labels
> > > ARM: dts: bcm2711: Add vmmc regulator in emmc2
> > >
> > > arch/arm/boot/dts/bcm2711-rpi-4-b.dts | 13 ++++++++++++-
> > > drivers/mmc/host/sdhci-iproc.c | 17 ++++++++++++++++-
> > > drivers/mmc/host/sdhci-milbeaut.c | 13 +------------
> > > drivers/mmc/host/sdhci-of-arasan.c | 15 ++-------------
> > > drivers/mmc/host/sdhci-of-at91.c | 18 +-----------------
> > > drivers/mmc/host/sdhci-pxav3.c | 20 +-------------------
> > > drivers/mmc/host/sdhci-xenon.c | 20 +-------------------
> > > drivers/mmc/host/sdhci.c | 24 +++++++++++++++++++++---
> > > drivers/mmc/host/sdhci.h | 5 +++--
> > > drivers/mmc/host/sdhci_am654.c | 17 +++--------------
> > > 10 files changed, 61 insertions(+), 101 deletions(-)
> > >
> > > --
> > > 2.25.1
> > >
> >
> > Patch 1-4, 6, 9 applied for next, thanks!
>
> I think you meant to apply 1-4, 7 and 9. Patch 6 is one of the contentious
> ones.
Yes, that's what I did, but told you about 6. :-)
Thanks and sorry for the noise.
Kind regards
Uffe
On Fri, 2020-03-06 at 18:44 +0100, Nicolas Saenz Julienne wrote:
> The SD card power can be controlled trough a pin routed into the board's
> external GPIO expander. Turn that into a regulator and provide it to
> emmc2.
>
> Signed-off-by: Nicolas Saenz Julienne <[email protected]>
Applied for-next.
Regards,
Nicolas
On Fri, 2020-03-06 at 18:44 +0100, Nicolas Saenz Julienne wrote:
> The 6th line of the GPIO expander is used to power the board's SD card.
>
> Signed-off-by: Nicolas Saenz Julienne <[email protected]>
> ---
Applied for-next.
Regards,
Nicolas