[PATCH v2 0/6] Add support of inverting power control and some minor cleanup

[PATCH v2 0/6] Add support of inverting power control and some minor cleanup

[Shawn Lin]

By default, dw_mmc outputs high level voltage to indicate powering
up the card and outputs low level vcltage to indicate powering
off the card. But that is not always correct. The power io should
be able to control different kind of hw components to supply or
cutoff power to the card. We have boards that need this patchset
to make the power control correct. Meanwhile let's expose it to
DT for board-specific usage.


Changes in v2:
- fix copy-paste err and typo

Shawn Lin (6):
  dt-bindings: rockchip-dw-mshc: add description of
    rockchip,power-invert
  mmc: dw_mmc: cleanup power setting of set_ios callback
  mmc: dw_mmc: split out dw_mci_set_power
  mmc: dw_mmc: split out dw_mci_set_power_reg
  mmc: dw_mmc: support inverted power control
  mmc: dw_mmc-rockchip: add parsing of power control from DT

 .../devicetree/bindings/mmc/rockchip-dw-mshc.txt   |   6 +
 drivers/mmc/host/dw_mmc-rockchip.c                 |   8 ++
 drivers/mmc/host/dw_mmc.c                          | 134 ++++++++++++---------
 drivers/mmc/host/dw_mmc.h                          |   1 +
 4 files changed, 90 insertions(+), 59 deletions(-)

-- 
2.3.7


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 1/6] dt-bindings: rockchip-dw-mshc: add description of rockchip,power-invert

[Shawn Lin]

rockchip,power-invert is used for rockchip to introduce the flag
DW_MMC_CARD_PWR_INVERT from DT which should make the power control
of dw_mmc more flexible according to different hw design.

Signed-off-by: Shawn Lin <shawn.lin-TNX95d0MmH7DzftRWevZcw@public.gmane.org>

---

Changes in v2: None

 Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt b/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt
index 07184e8..5640659 100644
--- a/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt
+++ b/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt
@@ -30,6 +30,12 @@ Optional Properties:
   probing, low speeds or in case where all phases work at tuning time.
   If not specified 0 deg will be used.
 
+* rockchip,power-invert: Invert the power control of PWREN register. If adding
+  it, the card should be in power-on state when the power io outputs high level
+  voltage, and it should be in power-off state when outputing low level voltage.
+  otherwise the behaviour of power control should be the opposite way which is
+  the default policy of dw mshc.
+
 Example:
 
 	rkdwmmc0@12200000 {
-- 
2.3.7


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/6] dt-bindings: rockchip-dw-mshc: add description of rockchip,power-invert

[Rob Herring]

On Sun, Aug 07, 2016 at 09:34:04AM +0800, Shawn Lin wrote:
> rockchip,power-invert is used for rockchip to introduce the flag
> DW_MMC_CARD_PWR_INVERT from DT which should make the power control
> of dw_mmc more flexible according to different hw design.
> 
> Signed-off-by: Shawn Lin <shawn.lin-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
> 
> ---
> 
> Changes in v2: None
> 
>  Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt | 6 ++++++
>  1 file changed, 6 insertions(+)

Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

[PATCH v2 3/6] mmc: dw_mmc: split out dw_mci_set_power

[Shawn Lin]

The dw_mci_set_ios is a little complicated and lengthy,
let's split out the setting of power part. No functionality
change.

Signed-off-by: Shawn Lin <shawn.lin-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
---

Changes in v2: None

 drivers/mmc/host/dw_mmc.c | 87 ++++++++++++++++++++++++++---------------------
 1 file changed, 49 insertions(+), 38 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 59a5e9c..0e5ed23 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -1276,54 +1276,19 @@ static void dw_mci_request(struct mmc_host *mmc, struct mmc_request *mrq)
 	spin_unlock_bh(&host->lock);
 }
 
-static void dw_mci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
+static int dw_mci_set_power(struct mmc_host *mmc, struct mmc_ios *ios)
 {
 	struct dw_mci_slot *slot = mmc_priv(mmc);
-	const struct dw_mci_drv_data *drv_data = slot->host->drv_data;
 	u32 regs;
 
-	switch (ios->bus_width) {
-	case MMC_BUS_WIDTH_4:
-		slot->ctype = SDMMC_CTYPE_4BIT;
-		break;
-	case MMC_BUS_WIDTH_8:
-		slot->ctype = SDMMC_CTYPE_8BIT;
-		break;
-	default:
-		/* set default 1 bit mode */
-		slot->ctype = SDMMC_CTYPE_1BIT;
-	}
-
-	regs = mci_readl(slot->host, UHS_REG);
-
-	/* DDR mode set */
-	if (ios->timing == MMC_TIMING_MMC_DDR52 ||
-	    ios->timing == MMC_TIMING_UHS_DDR50 ||
-	    ios->timing == MMC_TIMING_MMC_HS400)
-		regs |= ((0x1 << slot->id) << 16);
-	else
-		regs &= ~((0x1 << slot->id) << 16);
-
-	mci_writel(slot->host, UHS_REG, regs);
-	slot->host->timing = ios->timing;
-
-	/*
-	 * Use mirror of ios->clock to prevent race with mmc
-	 * core ios update when finding the minimum.
-	 */
-	slot->clock = ios->clock;
-
-	if (drv_data && drv_data->set_ios)
-		drv_data->set_ios(slot->host, ios);
-
 	switch (ios->power_mode) {
 	case MMC_POWER_UP:
 		if (!IS_ERR(mmc->supply.vmmc) &&
 		    mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, ios->vdd)) {
 			dev_err(slot->host->dev,
 				"failed to enable vmmc regulator\n");
-			/*return, if failed turn on vmmc*/
-			return;
+
+			return -EINVAL;
 		}
 		set_bit(DW_MMC_CARD_NEED_INIT, &slot->flags);
 		regs = mci_readl(slot->host, PWREN);
@@ -1369,6 +1334,52 @@ static void dw_mci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 		break;
 	}
 
+	return 0;
+}
+
+static void dw_mci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
+{
+	struct dw_mci_slot *slot = mmc_priv(mmc);
+	const struct dw_mci_drv_data *drv_data = slot->host->drv_data;
+	u32 regs;
+
+	switch (ios->bus_width) {
+	case MMC_BUS_WIDTH_4:
+		slot->ctype = SDMMC_CTYPE_4BIT;
+		break;
+	case MMC_BUS_WIDTH_8:
+		slot->ctype = SDMMC_CTYPE_8BIT;
+		break;
+	default:
+		/* set default 1 bit mode */
+		slot->ctype = SDMMC_CTYPE_1BIT;
+	}
+
+	regs = mci_readl(slot->host, UHS_REG);
+
+	/* DDR mode set */
+	if (ios->timing == MMC_TIMING_MMC_DDR52 ||
+	    ios->timing == MMC_TIMING_UHS_DDR50 ||
+	    ios->timing == MMC_TIMING_MMC_HS400)
+		regs |= ((0x1 << slot->id) << 16);
+	else
+		regs &= ~((0x1 << slot->id) << 16);
+
+	mci_writel(slot->host, UHS_REG, regs);
+	slot->host->timing = ios->timing;
+
+	/*
+	 * Use mirror of ios->clock to prevent race with mmc
+	 * core ios update when finding the minimum.
+	 */
+	slot->clock = ios->clock;
+
+	if (drv_data && drv_data->set_ios)
+		drv_data->set_ios(slot->host, ios);
+
+	if (dw_mci_set_power(mmc, ios))
+		return;
+
 	if (slot->host->state == STATE_WAITING_CMD11_DONE && ios->clock != 0)
 		slot->host->state = STATE_IDLE;
 }
-- 
2.3.7


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 2/6] mmc: dw_mmc: cleanup power setting of set_ios callback

[Shawn Lin]

Remove the ret variable and combine the condition check
to simplify the code.

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---

Changes in v2: None

 drivers/mmc/host/dw_mmc.c | 28 ++++++++++------------------
 1 file changed, 10 insertions(+), 18 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 32380d5..59a5e9c 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -1281,7 +1281,6 @@ static void dw_mci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 	struct dw_mci_slot *slot = mmc_priv(mmc);
 	const struct dw_mci_drv_data *drv_data = slot->host->drv_data;
 	u32 regs;
-	int ret;
 
 	switch (ios->bus_width) {
 	case MMC_BUS_WIDTH_4:
@@ -1319,15 +1318,12 @@ static void dw_mci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 
 	switch (ios->power_mode) {
 	case MMC_POWER_UP:
-		if (!IS_ERR(mmc->supply.vmmc)) {
-			ret = mmc_regulator_set_ocr(mmc, mmc->supply.vmmc,
-					ios->vdd);
-			if (ret) {
-				dev_err(slot->host->dev,
-					"failed to enable vmmc regulator\n");
-				/*return, if failed turn on vmmc*/
-				return;
-			}
+		if (!IS_ERR(mmc->supply.vmmc) &&
+		    mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, ios->vdd)) {
+			dev_err(slot->host->dev,
+				"failed to enable vmmc regulator\n");
+			/*return, if failed turn on vmmc*/
+			return;
 		}
 		set_bit(DW_MMC_CARD_NEED_INIT, &slot->flags);
 		regs = mci_readl(slot->host, PWREN);
@@ -1336,14 +1332,10 @@ static void dw_mci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 		break;
 	case MMC_POWER_ON:
 		if (!slot->host->vqmmc_enabled) {
-			if (!IS_ERR(mmc->supply.vqmmc)) {
-				ret = regulator_enable(mmc->supply.vqmmc);
-				if (ret < 0)
-					dev_err(slot->host->dev,
-						"failed to enable vqmmc\n");
-				else
-					slot->host->vqmmc_enabled = true;
-
+			if (!IS_ERR(mmc->supply.vqmmc) &&
+			    regulator_enable(mmc->supply.vqmmc)) {
+				dev_err(slot->host->dev,
+					"failed to enable vqmmc\n");
 			} else {
 				/* Keep track so we don't reset again */
 				slot->host->vqmmc_enabled = true;
-- 
2.3.7

[PATCH v2 4/6] mmc: dw_mmc: split out dw_mci_set_power_reg

[Shawn Lin]

Splite out dw_mci_set_power_reg to set PWREN register
in case that we need more complicated power control.

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---

Changes in v2: None

 drivers/mmc/host/dw_mmc.c | 26 ++++++++++++++++++--------
 1 file changed, 18 insertions(+), 8 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 0e5ed23..c35a26a 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -1276,10 +1276,25 @@ static void dw_mci_request(struct mmc_host *mmc, struct mmc_request *mrq)
 	spin_unlock_bh(&host->lock);
 }
 
+static inline void dw_mci_set_power_reg(struct dw_mci_slot *slot,
+					bool enable)
+{
+	u32 regs;
+
+	if (enable) {
+		regs = mci_readl(slot->host, PWREN);
+		regs |= (1 << slot->id);
+		mci_writel(slot->host, PWREN, regs);
+	} else {
+		regs = mci_readl(slot->host, PWREN);
+		regs &= ~(1 << slot->id);
+		mci_writel(slot->host, PWREN, regs);
+	}
+}
+
 static int dw_mci_set_power(struct mmc_host *mmc, struct mmc_ios *ios)
 {
 	struct dw_mci_slot *slot = mmc_priv(mmc);
-	u32 regs;
 
 	switch (ios->power_mode) {
 	case MMC_POWER_UP:
@@ -1291,9 +1306,7 @@ static int dw_mci_set_power(struct mmc_host *mmc, struct mmc_ios *ios)
 			return -EINVAL;
 		}
 		set_bit(DW_MMC_CARD_NEED_INIT, &slot->flags);
-		regs = mci_readl(slot->host, PWREN);
-		regs |= (1 << slot->id);
-		mci_writel(slot->host, PWREN, regs);
+		dw_mci_set_power_reg(slot, true);
 		break;
 	case MMC_POWER_ON:
 		if (!slot->host->vqmmc_enabled) {
@@ -1325,10 +1338,7 @@ static int dw_mci_set_power(struct mmc_host *mmc, struct mmc_ios *ios)
 		if (!IS_ERR(mmc->supply.vqmmc) && slot->host->vqmmc_enabled)
 			regulator_disable(mmc->supply.vqmmc);
 		slot->host->vqmmc_enabled = false;
-
-		regs = mci_readl(slot->host, PWREN);
-		regs &= ~(1 << slot->id);
-		mci_writel(slot->host, PWREN, regs);
+		dw_mci_set_power_reg(slot, false);
 		break;
 	default:
 		break;
-- 
2.3.7

[PATCH v2 5/6] mmc: dw_mmc: support inverted power control

[Shawn Lin]

If DW_MMC_CARD_PWR_INVERT is set, we should configure
PWREN register on the opposite way as it means we need to
output high level from the power io to indicate the off state.

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---

Changes in v2: None

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

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index c35a26a..20d786c 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -1281,6 +1281,9 @@ static inline void dw_mci_set_power_reg(struct dw_mci_slot *slot,
 {
 	u32 regs;
 
+	if (test_bit(DW_MMC_CARD_PWR_INVERT, &slot->flags))
+		enable = !enable;
+
 	if (enable) {
 		regs = mci_readl(slot->host, PWREN);
 		regs |= (1 << slot->id);
diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
index 9e740bc..5719fd1 100644
--- a/drivers/mmc/host/dw_mmc.h
+++ b/drivers/mmc/host/dw_mmc.h
@@ -269,6 +269,7 @@ struct dw_mci_slot {
 #define DW_MMC_CARD_NEED_INIT	1
 #define DW_MMC_CARD_NO_LOW_PWR	2
 #define DW_MMC_CARD_NO_USE_HOLD 3
+#define DW_MMC_CARD_PWR_INVERT	4
 	int			id;
 	int			sdio_id;
 };
-- 
2.3.7

[PATCH v2 6/6] mmc: dw_mmc-rockchip: add parsing of power control from DT

[Shawn Lin]

By default, the power io of dw_mmc is fixed which means we could
only output high level voltage to indicate the power-on state.
But that is not always correct as the usage of power io should
be board specific. Let's expose it to dt for supporting this
kind of specific design.

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>

---

Changes in v2:
- fix copy-paste err and typo

 drivers/mmc/host/dw_mmc-rockchip.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/mmc/host/dw_mmc-rockchip.c b/drivers/mmc/host/dw_mmc-rockchip.c
index 25eae35..2e51202 100644
--- a/drivers/mmc/host/dw_mmc-rockchip.c
+++ b/drivers/mmc/host/dw_mmc-rockchip.c
@@ -24,6 +24,7 @@ struct dw_mci_rockchip_priv_data {
 	struct clk		*drv_clk;
 	struct clk		*sample_clk;
 	int			default_sample_phase;
+	bool			power_invert;
 };
 
 static void dw_mci_rk3288_set_ios(struct dw_mci *host, struct mmc_ios *ios)
@@ -67,6 +68,10 @@ static void dw_mci_rk3288_set_ios(struct dw_mci *host, struct mmc_ios *ios)
 	if (!IS_ERR(priv->sample_clk))
 		clk_set_phase(priv->sample_clk, priv->default_sample_phase);
 
+	/* Make sure we need to invert the output of PWREN */
+	if (priv->power_invert)
+		set_bit(DW_MMC_CARD_PWR_INVERT, &host->cur_slot->flags);
+
 	/*
 	 * Set the drive phase offset based on speed mode to achieve hold times.
 	 *
@@ -267,6 +272,9 @@ static int dw_mci_rk3288_parse_dt(struct dw_mci *host)
 					&priv->default_sample_phase))
 		priv->default_sample_phase = 0;
 
+	if (of_property_read_bool(np, "rockchip,power-invert"))
+		priv->power_invert = true;
+
 	priv->drv_clk = devm_clk_get(host->dev, "ciu-drive");
 	if (IS_ERR(priv->drv_clk))
 		dev_dbg(host->dev, "ciu_drv not available\n");
-- 
2.3.7

Re: [PATCH v2 0/6] Add support of inverting power control and some minor cleanup

[Jaehoon Chung]

Hi Shawn,

On 08/07/2016 10:33 AM, Shawn Lin wrote:
> By default, dw_mmc outputs high level voltage to indicate powering
> up the card and outputs low level vcltage to indicate powering
> off the card. But that is not always correct. The power io should
> be able to control different kind of hw components to supply or
> cutoff power to the card. We have boards that need this patchset
> to make the power control correct. Meanwhile let's expose it to
> DT for board-specific usage.

I have a question for this patch-set. Does DWMMC IP support to invert ON/OFF at Power Enable register?
Hmm..Well, if use the DW_MMC_CARD_PWR_INVERT, it should also be the similar behavior with Quirks.

Other flags are related with dwmmc IP. But this flag (DW_MMC_CARD_PWR_INVERT) is not related with IP side.
I understood why you needs to add this flag..Is rockchip designed to invert the power controlling?

But it's not general case. We can discuss about this.

Best Regards,
Jaehoon Chung

> 
> 
> Changes in v2:
> - fix copy-paste err and typo
> 
> Shawn Lin (6):
>   dt-bindings: rockchip-dw-mshc: add description of
>     rockchip,power-invert
>   mmc: dw_mmc: cleanup power setting of set_ios callback
>   mmc: dw_mmc: split out dw_mci_set_power
>   mmc: dw_mmc: split out dw_mci_set_power_reg
>   mmc: dw_mmc: support inverted power control
>   mmc: dw_mmc-rockchip: add parsing of power control from DT
> 
>  .../devicetree/bindings/mmc/rockchip-dw-mshc.txt   |   6 +
>  drivers/mmc/host/dw_mmc-rockchip.c                 |   8 ++
>  drivers/mmc/host/dw_mmc.c                          | 134 ++++++++++++---------
>  drivers/mmc/host/dw_mmc.h                          |   1 +
>  4 files changed, 90 insertions(+), 59 deletions(-)
> 

Re: [PATCH v2 0/6] Add support of inverting power control and some minor cleanup

[Shawn Lin]

Hi,

On 2016/8/8 18:24, Jaehoon Chung wrote:
> Hi Shawn,
>
> On 08/07/2016 10:33 AM, Shawn Lin wrote:
>> By default, dw_mmc outputs high level voltage to indicate powering
>> up the card and outputs low level vcltage to indicate powering
>> off the card. But that is not always correct. The power io should
>> be able to control different kind of hw components to supply or
>> cutoff power to the card. We have boards that need this patchset
>> to make the power control correct. Meanwhile let's expose it to
>> DT for board-specific usage.
>
> I have a question for this patch-set. Does DWMMC IP support to invert ON/OFF at Power Enable register?

No.

> Hmm..Well, if use the DW_MMC_CARD_PWR_INVERT, it should also be the similar behavior with Quirks.

yup, it makes the power control more complicated than before. :(

>
> Other flags are related with dwmmc IP. But this flag (DW_MMC_CARD_PWR_INVERT) is not related with IP side.
> I understood why you needs to add this flag..Is rockchip designed to invert the power controlling?

We don't invert the power controlling but our customers do.
The HW componet looks like some discrete LDOs which enable the related
power supply when outputing low voltage from pwren..

>
> But it's not general case. We can discuss about this.

I have a solution which is to add gpio power control for slot-gpio of
mmc core. once finished, we could add pwr_cap_invert just like what we
did for cd/wp invert control..

Then we could remove PWREN from the default state of
pinctrl inside the sdmmc dt node, and let dwmmc request gpio power
control stuff after paring the property for pwr_cap_invert..

More over, it well fit for all mmc host's requirement of gpio
power control and inverted control if they want it. :)


That should be legit for us?
If it sounds ok to you and Ulf, I will come up with a RFC one for
community to comment it.:)

>
> Best Regards,
> Jaehoon Chung
>
>>
>>
>> Changes in v2:
>> - fix copy-paste err and typo
>>
>> Shawn Lin (6):
>>   dt-bindings: rockchip-dw-mshc: add description of
>>     rockchip,power-invert
>>   mmc: dw_mmc: cleanup power setting of set_ios callback
>>   mmc: dw_mmc: split out dw_mci_set_power
>>   mmc: dw_mmc: split out dw_mci_set_power_reg
>>   mmc: dw_mmc: support inverted power control
>>   mmc: dw_mmc-rockchip: add parsing of power control from DT
>>
>>  .../devicetree/bindings/mmc/rockchip-dw-mshc.txt   |   6 +
>>  drivers/mmc/host/dw_mmc-rockchip.c                 |   8 ++
>>  drivers/mmc/host/dw_mmc.c                          | 134 ++++++++++++---------
>>  drivers/mmc/host/dw_mmc.h                          |   1 +
>>  4 files changed, 90 insertions(+), 59 deletions(-)
>>
>
>
>
>


-- 
Best Regards
Shawn Lin

Re: [PATCH v2 0/6] Add support of inverting power control and some minor cleanup

[Shawn Lin]

On 2016/8/9 9:49, Shawn Lin wrote:
> Hi,
>
> On 2016/8/8 18:24, Jaehoon Chung wrote:
>> Hi Shawn,
>>
>> On 08/07/2016 10:33 AM, Shawn Lin wrote:
>>> By default, dw_mmc outputs high level voltage to indicate powering
>>> up the card and outputs low level vcltage to indicate powering
>>> off the card. But that is not always correct. The power io should
>>> be able to control different kind of hw components to supply or
>>> cutoff power to the card. We have boards that need this patchset
>>> to make the power control correct. Meanwhile let's expose it to
>>> DT for board-specific usage.
>>
>> I have a question for this patch-set. Does DWMMC IP support to invert
>> ON/OFF at Power Enable register?
>
> No.
>
>> Hmm..Well, if use the DW_MMC_CARD_PWR_INVERT, it should also be the
>> similar behavior with Quirks.
>
> yup, it makes the power control more complicated than before. :(
>
>>
>> Other flags are related with dwmmc IP. But this flag
>> (DW_MMC_CARD_PWR_INVERT) is not related with IP side.
>> I understood why you needs to add this flag..Is rockchip designed to
>> invert the power controlling?
>
> We don't invert the power controlling but our customers do.
> The HW componet looks like some discrete LDOs which enable the related
> power supply when outputing low voltage from pwren..
>
>>
>> But it's not general case. We can discuss about this.
>
> I have a solution which is to add gpio power control for slot-gpio of
> mmc core. once finished, we could add pwr_cap_invert just like what we
> did for cd/wp invert control..
>
> Then we could remove PWREN from the default state of
> pinctrl inside the sdmmc dt node, and let dwmmc request gpio power
> control stuff after paring the property for pwr_cap_invert..
>
> More over, it well fit for all mmc host's requirement of gpio
> power control and inverted control if they want it. :)
>
>
> That should be legit for us?
> If it sounds ok to you and Ulf, I will come up with a RFC one for
> community to comment it.:)

one more, maybe pwrseq is also a choice for us.
But I'm not sure if it deserves a pwerseq_sd.c ?

>
>>
>> Best Regards,
>> Jaehoon Chung
>>
>>>
>>>
>>> Changes in v2:
>>> - fix copy-paste err and typo
>>>
>>> Shawn Lin (6):
>>>   dt-bindings: rockchip-dw-mshc: add description of
>>>     rockchip,power-invert
>>>   mmc: dw_mmc: cleanup power setting of set_ios callback
>>>   mmc: dw_mmc: split out dw_mci_set_power
>>>   mmc: dw_mmc: split out dw_mci_set_power_reg
>>>   mmc: dw_mmc: support inverted power control
>>>   mmc: dw_mmc-rockchip: add parsing of power control from DT
>>>
>>>  .../devicetree/bindings/mmc/rockchip-dw-mshc.txt   |   6 +
>>>  drivers/mmc/host/dw_mmc-rockchip.c                 |   8 ++
>>>  drivers/mmc/host/dw_mmc.c                          | 134
>>> ++++++++++++---------
>>>  drivers/mmc/host/dw_mmc.h                          |   1 +
>>>  4 files changed, 90 insertions(+), 59 deletions(-)
>>>
>>
>>
>>
>>
>
>


-- 
Best Regards
Shawn Lin

Re: [PATCH v2 0/6] Add support of inverting power control and some minor cleanup

[Jaehoon Chung]

Hi Shawn,

On 08/09/2016 10:49 AM, Shawn Lin wrote:
> Hi,
> 
> On 2016/8/8 18:24, Jaehoon Chung wrote:
>> Hi Shawn,
>>
>> On 08/07/2016 10:33 AM, Shawn Lin wrote:
>>> By default, dw_mmc outputs high level voltage to indicate powering
>>> up the card and outputs low level vcltage to indicate powering
>>> off the card. But that is not always correct. The power io should
>>> be able to control different kind of hw components to supply or
>>> cutoff power to the card. We have boards that need this patchset
>>> to make the power control correct. Meanwhile let's expose it to
>>> DT for board-specific usage.
>>
>> I have a question for this patch-set. Does DWMMC IP support to invert ON/OFF at Power Enable register?
> 
> No.
> 
>> Hmm..Well, if use the DW_MMC_CARD_PWR_INVERT, it should also be the similar behavior with Quirks.
> 
> yup, it makes the power control more complicated than before. :(
> 
>>
>> Other flags are related with dwmmc IP. But this flag (DW_MMC_CARD_PWR_INVERT) is not related with IP side.
>> I understood why you needs to add this flag..Is rockchip designed to invert the power controlling?
> 
> We don't invert the power controlling but our customers do.
> The HW componet looks like some discrete LDOs which enable the related
> power supply when outputing low voltage from pwren..

Is it related with PWREN register?
I'm checking the TRM for PWREN register..

"Optional feature; ports can be used as general-purpose outputs."

Does it use the ports as GPIO?

> 
>>
>> But it's not general case. We can discuss about this.
> 
> I have a solution which is to add gpio power control for slot-gpio of
> mmc core. once finished, we could add pwr_cap_invert just like what we
> did for cd/wp invert control..

I'm not sure that it's right that add the gpio power control into mmc core.
slot-gpio? i think it can use the regulator framework with gpio.
I don't have a lot knowledge of power, but it might be controlled whether active is low or high.
So i think it doesn't need to add for entire mmc controller.

Best Regards,
Jaehoon Chung

> 
> Then we could remove PWREN from the default state of
> pinctrl inside the sdmmc dt node, and let dwmmc request gpio power
> control stuff after paring the property for pwr_cap_invert..
> 
> More over, it well fit for all mmc host's requirement of gpio
> power control and inverted control if they want it. :)
> 
> 
> That should be legit for us?
> If it sounds ok to you and Ulf, I will come up with a RFC one for
> community to comment it.:)
> 
>>
>> Best Regards,
>> Jaehoon Chung
>>
>>>
>>>
>>> Changes in v2:
>>> - fix copy-paste err and typo
>>>
>>> Shawn Lin (6):
>>>   dt-bindings: rockchip-dw-mshc: add description of
>>>     rockchip,power-invert
>>>   mmc: dw_mmc: cleanup power setting of set_ios callback
>>>   mmc: dw_mmc: split out dw_mci_set_power
>>>   mmc: dw_mmc: split out dw_mci_set_power_reg
>>>   mmc: dw_mmc: support inverted power control
>>>   mmc: dw_mmc-rockchip: add parsing of power control from DT
>>>
>>>  .../devicetree/bindings/mmc/rockchip-dw-mshc.txt   |   6 +
>>>  drivers/mmc/host/dw_mmc-rockchip.c                 |   8 ++
>>>  drivers/mmc/host/dw_mmc.c                          | 134 ++++++++++++---------
>>>  drivers/mmc/host/dw_mmc.h                          |   1 +
>>>  4 files changed, 90 insertions(+), 59 deletions(-)
>>>
>>
>>
>>
>>
> 
> 

Re: [PATCH v2 0/6] Add support of inverting power control and some minor cleanup

[Shawn Lin]

On 2016/8/11 18:08, Jaehoon Chung wrote:
> Hi Shawn,
>
> On 08/09/2016 10:49 AM, Shawn Lin wrote:
>> Hi,
>>
>> On 2016/8/8 18:24, Jaehoon Chung wrote:
>>> Hi Shawn,
>>>
>>> On 08/07/2016 10:33 AM, Shawn Lin wrote:
>>>> By default, dw_mmc outputs high level voltage to indicate powering
>>>> up the card and outputs low level vcltage to indicate powering
>>>> off the card. But that is not always correct. The power io should
>>>> be able to control different kind of hw components to supply or
>>>> cutoff power to the card. We have boards that need this patchset
>>>> to make the power control correct. Meanwhile let's expose it to
>>>> DT for board-specific usage.
>>>
>>> I have a question for this patch-set. Does DWMMC IP support to invert ON/OFF at Power Enable register?
>>
>> No.
>>
>>> Hmm..Well, if use the DW_MMC_CARD_PWR_INVERT, it should also be the similar behavior with Quirks.
>>
>> yup, it makes the power control more complicated than before. :(
>>
>>>
>>> Other flags are related with dwmmc IP. But this flag (DW_MMC_CARD_PWR_INVERT) is not related with IP side.
>>> I understood why you needs to add this flag..Is rockchip designed to invert the power controlling?
>>
>> We don't invert the power controlling but our customers do.
>> The HW componet looks like some discrete LDOs which enable the related
>> power supply when outputing low voltage from pwren..
>
> Is it related with PWREN register?
> I'm checking the TRM for PWREN register..
>
> "Optional feature; ports can be used as general-purpose outputs."
>
> Does it use the ports as GPIO?
>
>>
>>>
>>> But it's not general case. We can discuss about this.
>>
>> I have a solution which is to add gpio power control for slot-gpio of
>> mmc core. once finished, we could add pwr_cap_invert just like what we
>> did for cd/wp invert control..
>
> I'm not sure that it's right that add the gpio power control into mmc core.
> slot-gpio? i think it can use the regulator framework with gpio.

yup, I'm looking this and seems it's fine to register a regulator-gpio
and stuff it for vmmc-supply of sdmmc. I will test it, really helpful.

thanks!

> I don't have a lot knowledge of power, but it might be controlled whether active is low or high.
> So i think it doesn't need to add for entire mmc controller.
>
> Best Regards,
> Jaehoon Chung
>
>>
>> Then we could remove PWREN from the default state of
>> pinctrl inside the sdmmc dt node, and let dwmmc request gpio power
>> control stuff after paring the property for pwr_cap_invert..
>>
>> More over, it well fit for all mmc host's requirement of gpio
>> power control and inverted control if they want it. :)
>>
>>
>> That should be legit for us?
>> If it sounds ok to you and Ulf, I will come up with a RFC one for
>> community to comment it.:)
>>
>>>
>>> Best Regards,
>>> Jaehoon Chung
>>>
>>>>
>>>>
>>>> Changes in v2:
>>>> - fix copy-paste err and typo
>>>>
>>>> Shawn Lin (6):
>>>>   dt-bindings: rockchip-dw-mshc: add description of
>>>>     rockchip,power-invert
>>>>   mmc: dw_mmc: cleanup power setting of set_ios callback
>>>>   mmc: dw_mmc: split out dw_mci_set_power
>>>>   mmc: dw_mmc: split out dw_mci_set_power_reg
>>>>   mmc: dw_mmc: support inverted power control
>>>>   mmc: dw_mmc-rockchip: add parsing of power control from DT
>>>>
>>>>  .../devicetree/bindings/mmc/rockchip-dw-mshc.txt   |   6 +
>>>>  drivers/mmc/host/dw_mmc-rockchip.c                 |   8 ++
>>>>  drivers/mmc/host/dw_mmc.c                          | 134 ++++++++++++---------
>>>>  drivers/mmc/host/dw_mmc.h                          |   1 +
>>>>  4 files changed, 90 insertions(+), 59 deletions(-)
>>>>
>>>
>>>
>>>
>>>
>>
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>


-- 
Best Regards
Shawn Lin