[PATCH v2 0/8] mmc: dw_mmc-rockchip: allow tuning using the clk-phase api

[PATCH v2 0/8] mmc: dw_mmc-rockchip: allow tuning using the clk-phase api

[Heiko Stuebner]

This series resurrects and adapts some individual patches whose sum
enable the dw_mmc hosts on Rockchip socs to tune clock phases using
the generic phase api (rk3288 and following have this capability).

The changes to the original mmc-phase clocks are expanded by further
findings resulting from devices being used in the field.

Similarly the regulator handling changes do use the brand new
regulator_set_voltage_triplet call to allow specifying lower and upper
limits. One possible point of discussion are the two voltage ranges
that are tried for the 3.3V signal level. Trying to stay near vmmc
at first and only then opening the range to the full 2.7-3.6V.

This mainly circumvents a shortcoming of the regulator voltage
setting, in that even with regulator_set_voltage_triplet the regulator
framework will take the lowest possible voltage when the possible
voltages are below the target voltage. While it may be ideal to solve
this on the regulator side, I'm not seeing this appearing in the short
term, mainly because all regulator parts (including regulator drivers)
are keyed to selecting the lowest voltage from the range, while on the
mmc side we know which voltages may work and trying this in two steps
does not create to much overhead, as unsupported voltages are already
filtered out by the regulator_is_voltage_selected calls.

changes since v1:
- address comment from Jaehoon Chung and keep this local to Rockchip
  for the time being
- address comments from Ulf and Doug in making it more explicit
  that the two-step 3.3V voltage setting essentially works around
  a limitation of regulator_set_voltage_triplet

Alexandru M Stan (3):
  mmc: dw_mmc-rockchip: dt-binding: Add tuning related things
  mmc: dw_mmc: Generic MMC tuning with the clock phase framework
  ARM: dts: rockchip: Add drive/sample clocks for rk3288 dw_mmc devices

Douglas Anderson (4):
  clk: rockchip: Allow more precision for some mmc clock phases
  clk: rockchip: Make calculations use rounding
  mmc: core: Add mmc_regulator_set_vqmmc()
  mmc: dw_mmc: Use mmc_regulator_set_vqmmc in
    start_signal_voltage_switch

Heiko Stuebner (1):
  ARM: dts: rockchip: add tuning related settings to veyron devices

 .../devicetree/bindings/mmc/rockchip-dw-mshc.txt   |  13 ++
 arch/arm/boot/dts/rk3288-veyron-sdmmc.dtsi         |   7 +-
 arch/arm/boot/dts/rk3288-veyron.dtsi               |   6 +
 arch/arm/boot/dts/rk3288.dtsi                      |  20 ++-
 drivers/clk/rockchip/clk-mmc-phase.c               |  54 ++++---
 drivers/mmc/core/core.c                            |  74 ++++++++++
 drivers/mmc/host/dw_mmc-rockchip.c                 | 162 +++++++++++++++++++++
 drivers/mmc/host/dw_mmc.c                          |  17 +--
 include/linux/mmc/host.h                           |   7 +
 9 files changed, 321 insertions(+), 39 deletions(-)

-- 
2.5.1

[PATCH v2 0/8] mmc: dw_mmc-rockchip: allow tuning using the clk-phase api

[Heiko Stuebner]

This series resurrects and adapts some individual patches whose sum
enable the dw_mmc hosts on Rockchip socs to tune clock phases using
the generic phase api (rk3288 and following have this capability).

The changes to the original mmc-phase clocks are expanded by further
findings resulting from devices being used in the field.

Similarly the regulator handling changes do use the brand new
regulator_set_voltage_triplet call to allow specifying lower and upper
limits. One possible point of discussion are the two voltage ranges
that are tried for the 3.3V signal level. Trying to stay near vmmc
at first and only then opening the range to the full 2.7-3.6V.

This mainly circumvents a shortcoming of the regulator voltage
setting, in that even with regulator_set_voltage_triplet the regulator
framework will take the lowest possible voltage when the possible
voltages are below the target voltage. While it may be ideal to solve
this on the regulator side, I'm not seeing this appearing in the short
term, mainly because all regulator parts (including regulator drivers)
are keyed to selecting the lowest voltage from the range, while on the
mmc side we know which voltages may work and trying this in two steps
does not create to much overhead, as unsupported voltages are already
filtered out by the regulator_is_voltage_selected calls.

changes since v1:
- address comment from Jaehoon Chung and keep this local to Rockchip
  for the time being
- address comments from Ulf and Doug in making it more explicit
  that the two-step 3.3V voltage setting essentially works around
  a limitation of regulator_set_voltage_triplet

Alexandru M Stan (3):
  mmc: dw_mmc-rockchip: dt-binding: Add tuning related things
  mmc: dw_mmc: Generic MMC tuning with the clock phase framework
  ARM: dts: rockchip: Add drive/sample clocks for rk3288 dw_mmc devices

Douglas Anderson (4):
  clk: rockchip: Allow more precision for some mmc clock phases
  clk: rockchip: Make calculations use rounding
  mmc: core: Add mmc_regulator_set_vqmmc()
  mmc: dw_mmc: Use mmc_regulator_set_vqmmc in
    start_signal_voltage_switch

Heiko Stuebner (1):
  ARM: dts: rockchip: add tuning related settings to veyron devices

 .../devicetree/bindings/mmc/rockchip-dw-mshc.txt   |  13 ++
 arch/arm/boot/dts/rk3288-veyron-sdmmc.dtsi         |   7 +-
 arch/arm/boot/dts/rk3288-veyron.dtsi               |   6 +
 arch/arm/boot/dts/rk3288.dtsi                      |  20 ++-
 drivers/clk/rockchip/clk-mmc-phase.c               |  54 ++++---
 drivers/mmc/core/core.c                            |  74 ++++++++++
 drivers/mmc/host/dw_mmc-rockchip.c                 | 162 +++++++++++++++++++++
 drivers/mmc/host/dw_mmc.c                          |  17 +--
 include/linux/mmc/host.h                           |   7 +
 9 files changed, 321 insertions(+), 39 deletions(-)

-- 
2.5.1

[PATCH v2 1/8] clk: rockchip: Allow more precision for some mmc clock phases

[Heiko Stuebner]

From: Douglas Anderson <dianders@chromium.org>

Because of the inexact nature of the extra MMC delay elements (it's
not possible to keep the phase monotonic and to also make phases (mod
90) > 70), we previously only allowed phases (mod 90) of 22.5, 45,
and 67.5.

But it's not the end of the world if the MMC clock phase goes
non-monotonic.  At most we'll be 25 degrees off.  It's way better to
test more phases to look for bad ones than to be 25 degrees off, because
in the case of MMC really the point is to find bad phases and get as far
asway from the as possible.  If we get to test extra phases by going
slightly non-monotonic then that might be fine.  Worst case we would
end up at a phases that's slight differnt than the one we wanted, but
at least we'd still be quite far away from the a bad phase.

Signed-off-by: Douglas Anderson <dianders@chromium.org>

Fold in more precise variance-values of 44-77 instead of 40-80.
Fold in the actual removal of the monotonic requirement and adapt
patch message accordingly.
Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
 drivers/clk/rockchip/clk-mmc-phase.c | 45 ++++++++++++++++++++++++------------
 1 file changed, 30 insertions(+), 15 deletions(-)

diff --git a/drivers/clk/rockchip/clk-mmc-phase.c b/drivers/clk/rockchip/clk-mmc-phase.c
index 9b61342..a797d86 100644
--- a/drivers/clk/rockchip/clk-mmc-phase.c
+++ b/drivers/clk/rockchip/clk-mmc-phase.c
@@ -45,8 +45,8 @@ static unsigned long rockchip_mmc_recalc(struct clk_hw *hw,
 #define PSECS_PER_SEC 1000000000000LL
 
 /*
- * Each fine delay is between 40ps-80ps. Assume each fine delay is 60ps to
- * simplify calculations. So 45degs could be anywhere between 33deg and 66deg.
+ * Each fine delay is between 44ps-77ps. Assume each fine delay is 60ps to
+ * simplify calculations. So 45degs could be anywhere between 33deg and 57.8deg.
  */
 #define ROCKCHIP_MMC_DELAY_ELEMENT_PSEC 60
 
@@ -84,22 +84,37 @@ static int rockchip_mmc_set_phase(struct clk_hw *hw, int degrees)
 	u32 raw_value;
 	u64 delay;
 
-	/* allow 22 to be 22.5 */
-	degrees++;
-	/* floor to 22.5 increment */
-	degrees -= ((degrees) * 10 % 225) / 10;
-
 	nineties = degrees / 90;
-	/* 22.5 multiples */
-	remainder = (degrees % 90) / 22;
-
+	remainder = (degrees % 90);
+
+	/*
+	 * Due to the inexact nature of the "fine" delay, we might
+	 * actually go non-monotonic.  We don't go _too_ monotonic
+	 * though, so we should be OK.  Here are options of how we may
+	 * work:
+	 *
+	 * Ideally we end up with:
+	 *   1.0, 2.0, ..., 69.0, 70.0, ...,  89.0, 90.0
+	 *
+	 * On one extreme (if delay is actually 44ps):
+	 *   .73, 1.5, ..., 50.6, 51.3, ...,  65.3, 90.0
+	 * The other (if delay is actually 77ps):
+	 *   1.3, 2.6, ..., 88.6. 89.8, ..., 114.0, 90
+	 *
+	 * It's possible we might make a delay that is up to 25
+	 * degrees off from what we think we're making.  That's OK
+	 * though because we should be REALLY far from any bad range.
+	 */
+
+	/*
+	 * Convert to delay; do a little extra work to make sure we
+	 * don't overflow 32-bit / 64-bit numbers.
+	 */
 	delay = PSECS_PER_SEC;
-	do_div(delay, rate);
-	/* / 360 / 22.5 */
-	do_div(delay, 16);
-	do_div(delay, ROCKCHIP_MMC_DELAY_ELEMENT_PSEC);
-
 	delay *= remainder;
+	do_div(delay, 10000);
+	do_div(delay, (rate / 1000) * 36 * ROCKCHIP_MMC_DELAY_ELEMENT_PSEC);
+
 	delay_num = (u8) min(delay, 255ULL);
 
 	raw_value = delay_num ? ROCKCHIP_MMC_DELAY_SEL : 0;
-- 
2.5.1

[PATCH v2 1/8] clk: rockchip: Allow more precision for some mmc clock phases

[Heiko Stuebner]

From: Douglas Anderson <dianders@chromium.org>

Because of the inexact nature of the extra MMC delay elements (it's
not possible to keep the phase monotonic and to also make phases (mod
90) > 70), we previously only allowed phases (mod 90) of 22.5, 45,
and 67.5.

But it's not the end of the world if the MMC clock phase goes
non-monotonic.  At most we'll be 25 degrees off.  It's way better to
test more phases to look for bad ones than to be 25 degrees off, because
in the case of MMC really the point is to find bad phases and get as far
asway from the as possible.  If we get to test extra phases by going
slightly non-monotonic then that might be fine.  Worst case we would
end up at a phases that's slight differnt than the one we wanted, but
at least we'd still be quite far away from the a bad phase.

Signed-off-by: Douglas Anderson <dianders@chromium.org>

Fold in more precise variance-values of 44-77 instead of 40-80.
Fold in the actual removal of the monotonic requirement and adapt
patch message accordingly.
Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
 drivers/clk/rockchip/clk-mmc-phase.c | 45 ++++++++++++++++++++++++------------
 1 file changed, 30 insertions(+), 15 deletions(-)

diff --git a/drivers/clk/rockchip/clk-mmc-phase.c b/drivers/clk/rockchip/clk-mmc-phase.c
index 9b61342..a797d86 100644
--- a/drivers/clk/rockchip/clk-mmc-phase.c
+++ b/drivers/clk/rockchip/clk-mmc-phase.c
@@ -45,8 +45,8 @@ static unsigned long rockchip_mmc_recalc(struct clk_hw *hw,
 #define PSECS_PER_SEC 1000000000000LL
 
 /*
- * Each fine delay is between 40ps-80ps. Assume each fine delay is 60ps to
- * simplify calculations. So 45degs could be anywhere between 33deg and 66deg.
+ * Each fine delay is between 44ps-77ps. Assume each fine delay is 60ps to
+ * simplify calculations. So 45degs could be anywhere between 33deg and 57.8deg.
  */
 #define ROCKCHIP_MMC_DELAY_ELEMENT_PSEC 60
 
@@ -84,22 +84,37 @@ static int rockchip_mmc_set_phase(struct clk_hw *hw, int degrees)
 	u32 raw_value;
 	u64 delay;
 
-	/* allow 22 to be 22.5 */
-	degrees++;
-	/* floor to 22.5 increment */
-	degrees -= ((degrees) * 10 % 225) / 10;
-
 	nineties = degrees / 90;
-	/* 22.5 multiples */
-	remainder = (degrees % 90) / 22;
-
+	remainder = (degrees % 90);
+
+	/*
+	 * Due to the inexact nature of the "fine" delay, we might
+	 * actually go non-monotonic.  We don't go _too_ monotonic
+	 * though, so we should be OK.  Here are options of how we may
+	 * work:
+	 *
+	 * Ideally we end up with:
+	 *   1.0, 2.0, ..., 69.0, 70.0, ...,  89.0, 90.0
+	 *
+	 * On one extreme (if delay is actually 44ps):
+	 *   .73, 1.5, ..., 50.6, 51.3, ...,  65.3, 90.0
+	 * The other (if delay is actually 77ps):
+	 *   1.3, 2.6, ..., 88.6. 89.8, ..., 114.0, 90
+	 *
+	 * It's possible we might make a delay that is up to 25
+	 * degrees off from what we think we're making.  That's OK
+	 * though because we should be REALLY far from any bad range.
+	 */
+
+	/*
+	 * Convert to delay; do a little extra work to make sure we
+	 * don't overflow 32-bit / 64-bit numbers.
+	 */
 	delay = PSECS_PER_SEC;
-	do_div(delay, rate);
-	/* / 360 / 22.5 */
-	do_div(delay, 16);
-	do_div(delay, ROCKCHIP_MMC_DELAY_ELEMENT_PSEC);
-
 	delay *= remainder;
+	do_div(delay, 10000);
+	do_div(delay, (rate / 1000) * 36 * ROCKCHIP_MMC_DELAY_ELEMENT_PSEC);
+
 	delay_num = (u8) min(delay, 255ULL);
 
 	raw_value = delay_num ? ROCKCHIP_MMC_DELAY_SEL : 0;
-- 
2.5.1

[PATCH v2 2/8] clk: rockchip: Make calculations use rounding

[Heiko Stuebner]

From: Douglas Anderson <dianders@chromium.org>

Let's use DIV_ROUND_CLOSEST for rounding, not just truncating
division.  This lets us get closer to the right rate.

Before this:
  set_phase(86) delay_nums=26 reg[0xf000420c]=0x468 actual_degrees=83
  set_phase(89) delay_nums=27 reg[0xf000420c]=0x46c actual_degrees=86

After this:
  set_phase(86) delay_nums=27 reg[0xf000420c]=0x46c actual_degrees=86
  set_phase(89) delay_nums=28 reg[0xf000420c]=0x470 actual_degrees=90

Signed-off-by: Douglas Anderson <dianders@chromium.org>
Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
 drivers/clk/rockchip/clk-mmc-phase.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/clk/rockchip/clk-mmc-phase.c b/drivers/clk/rockchip/clk-mmc-phase.c
index a797d86..bc24e5a 100644
--- a/drivers/clk/rockchip/clk-mmc-phase.c
+++ b/drivers/clk/rockchip/clk-mmc-phase.c
@@ -69,7 +69,7 @@ static int rockchip_mmc_get_phase(struct clk_hw *hw)
 
 		delay_num = (raw_value & ROCKCHIP_MMC_DELAYNUM_MASK);
 		delay_num >>= ROCKCHIP_MMC_DELAYNUM_OFFSET;
-		degrees += delay_num * factor / 10000;
+		degrees += DIV_ROUND_CLOSEST(delay_num * factor, 10000);
 	}
 
 	return degrees % 360;
@@ -82,7 +82,7 @@ static int rockchip_mmc_set_phase(struct clk_hw *hw, int degrees)
 	u8 nineties, remainder;
 	u8 delay_num;
 	u32 raw_value;
-	u64 delay;
+	u32 delay;
 
 	nineties = degrees / 90;
 	remainder = (degrees % 90);
@@ -110,12 +110,13 @@ static int rockchip_mmc_set_phase(struct clk_hw *hw, int degrees)
 	 * Convert to delay; do a little extra work to make sure we
 	 * don't overflow 32-bit / 64-bit numbers.
 	 */
-	delay = PSECS_PER_SEC;
+	delay = 10000000; /* PSECS_PER_SEC / 10000 / 10 */
 	delay *= remainder;
-	do_div(delay, 10000);
-	do_div(delay, (rate / 1000) * 36 * ROCKCHIP_MMC_DELAY_ELEMENT_PSEC);
+	delay = DIV_ROUND_CLOSEST(delay,
+			(rate / 1000) * 36 *
+				(ROCKCHIP_MMC_DELAY_ELEMENT_PSEC / 10));
 
-	delay_num = (u8) min(delay, 255ULL);
+	delay_num = (u8) min_t(u32, delay, 255);
 
 	raw_value = delay_num ? ROCKCHIP_MMC_DELAY_SEL : 0;
 	raw_value |= delay_num << ROCKCHIP_MMC_DELAYNUM_OFFSET;
-- 
2.5.1

[PATCH v2 2/8] clk: rockchip: Make calculations use rounding

[Heiko Stuebner]

From: Douglas Anderson <dianders@chromium.org>

Let's use DIV_ROUND_CLOSEST for rounding, not just truncating
division.  This lets us get closer to the right rate.

Before this:
  set_phase(86) delay_nums=26 reg[0xf000420c]=0x468 actual_degrees=83
  set_phase(89) delay_nums=27 reg[0xf000420c]=0x46c actual_degrees=86

After this:
  set_phase(86) delay_nums=27 reg[0xf000420c]=0x46c actual_degrees=86
  set_phase(89) delay_nums=28 reg[0xf000420c]=0x470 actual_degrees=90

Signed-off-by: Douglas Anderson <dianders@chromium.org>
Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
 drivers/clk/rockchip/clk-mmc-phase.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/clk/rockchip/clk-mmc-phase.c b/drivers/clk/rockchip/clk-mmc-phase.c
index a797d86..bc24e5a 100644
--- a/drivers/clk/rockchip/clk-mmc-phase.c
+++ b/drivers/clk/rockchip/clk-mmc-phase.c
@@ -69,7 +69,7 @@ static int rockchip_mmc_get_phase(struct clk_hw *hw)
 
 		delay_num = (raw_value & ROCKCHIP_MMC_DELAYNUM_MASK);
 		delay_num >>= ROCKCHIP_MMC_DELAYNUM_OFFSET;
-		degrees += delay_num * factor / 10000;
+		degrees += DIV_ROUND_CLOSEST(delay_num * factor, 10000);
 	}
 
 	return degrees % 360;
@@ -82,7 +82,7 @@ static int rockchip_mmc_set_phase(struct clk_hw *hw, int degrees)
 	u8 nineties, remainder;
 	u8 delay_num;
 	u32 raw_value;
-	u64 delay;
+	u32 delay;
 
 	nineties = degrees / 90;
 	remainder = (degrees % 90);
@@ -110,12 +110,13 @@ static int rockchip_mmc_set_phase(struct clk_hw *hw, int degrees)
 	 * Convert to delay; do a little extra work to make sure we
 	 * don't overflow 32-bit / 64-bit numbers.
 	 */
-	delay = PSECS_PER_SEC;
+	delay = 10000000; /* PSECS_PER_SEC / 10000 / 10 */
 	delay *= remainder;
-	do_div(delay, 10000);
-	do_div(delay, (rate / 1000) * 36 * ROCKCHIP_MMC_DELAY_ELEMENT_PSEC);
+	delay = DIV_ROUND_CLOSEST(delay,
+			(rate / 1000) * 36 *
+				(ROCKCHIP_MMC_DELAY_ELEMENT_PSEC / 10));
 
-	delay_num = (u8) min(delay, 255ULL);
+	delay_num = (u8) min_t(u32, delay, 255);
 
 	raw_value = delay_num ? ROCKCHIP_MMC_DELAY_SEL : 0;
 	raw_value |= delay_num << ROCKCHIP_MMC_DELAYNUM_OFFSET;
-- 
2.5.1

[PATCH v2 3/8] mmc: core: Add mmc_regulator_set_vqmmc()

[Heiko Stuebner]

From: Douglas Anderson <dianders@chromium.org>

This adds logic to the MMC core to set VQMMC.  This is expected to be
called by MMC drivers like dw_mmc as part of (or instead of) their
start_signal_voltage_switch() callback.

A few notes:

* When setting the signal voltage to 3.3V we do our best to make VQMMC
  and VMMC match.  It's been reported that this makes some old cards
  happy since they were tested back in the day before UHS when VQMMC
  and VMMC were provided by the same regulator.  A nice side effect of
  this is that we don't end up on the hairy edge of VQMMC (2.7V),
  which some EEs claim is a little too close to the minimum for
  comfort.
  This is done in two steps. At first we try to find a VQMMC within
  a 0.3V tolerance of VMMC and if this is not supported by the
  supplying regulator we try to find a suitable voltage within the
  whole 2.7V-3.6V area of the spec.

* The two step approach is currently necessary, as the used
  regulator_set_voltage_triplet(min, target, max) uses a simple
  implementation that just tries two basic steps:
	regulator_set_voltage(target, max);
	regulator_set_voltage(min, target);
  So with only one step with 2.7-3.6V borders, if a suitable voltage
  is a bit below VMMC, we would directly get the lowest 2.7V
  which some boards (like Rockchips) don't like at all.

* When setting the signal voltage to 1.8V or 1.2V we aim for that
  specific voltage instead of picking the lowest one in the range.

* We very purposely don't print errors in mmc_regulator_set_vqmmc().
  There are cases where the MMC core will try several different
  voltages and we don't want to pollute the logs.

Signed-off-by: Douglas Anderson <dianders@chromium.org>
Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
 drivers/mmc/core/core.c  | 74 ++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/mmc/host.h |  7 +++++
 2 files changed, 81 insertions(+)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 0520064..363cc98 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -1437,6 +1437,80 @@ int mmc_regulator_set_ocr(struct mmc_host *mmc,
 }
 EXPORT_SYMBOL_GPL(mmc_regulator_set_ocr);
 
+static int mmc_regulator_set_voltage_if_supported(struct regulator *regulator,
+						  int min_uV, int target_uV,
+						  int max_uV)
+{
+	/*
+	 * Check if supported first to avoid errors since we may try several
+	 * signal levels during power up and don't want to show errors.
+	 */
+	if (!regulator_is_supported_voltage(regulator, min_uV, max_uV))
+		return -EINVAL;
+
+	return regulator_set_voltage_triplet(regulator, min_uV, target_uV,
+					     max_uV);
+}
+
+/**
+ * mmc_regulator_set_vqmmc - Set VQMMC as per the ios
+ *
+ * For 3.3V signaling, we try to match VQMMC to VMMC as closely as possible.
+ * That will match the behavior of old boards where VQMMC and VMMC were supplied
+ * by the same supply.  The Bus Operating conditions for 3.3V signaling in the
+ * SD card spec also define VQMMC in terms of VMMC.
+ * If this is not possible we'll try the full 2.7-3.6V of the spec.
+ *
+ * For 1.2V and 1.8V signaling we'll try to get as close as possible to the
+ * requested voltage.  This is definitely a good idea for UHS where there's a
+ * separate regulator on the card that's trying to make 1.8V and it's best if
+ * we match.
+ *
+ * This function is expected to be used by a controller's
+ * start_signal_voltage_switch() function.
+ */
+int mmc_regulator_set_vqmmc(struct mmc_host *mmc, struct mmc_ios *ios)
+{
+	int volt, min_uV, max_uV;
+
+	/* If no vqmmc supply then we can't change the voltage */
+	if (IS_ERR(mmc->supply.vqmmc))
+		return -EINVAL;
+
+	switch (ios->signal_voltage) {
+	case MMC_SIGNAL_VOLTAGE_120:
+		return mmc_regulator_set_voltage_if_supported(mmc->supply.vqmmc,
+						1100000, 1200000, 1300000);
+	case MMC_SIGNAL_VOLTAGE_180:
+		return mmc_regulator_set_voltage_if_supported(mmc->supply.vqmmc,
+						1700000, 1800000, 1950000);
+	case MMC_SIGNAL_VOLTAGE_330:
+		volt = regulator_get_voltage(mmc->supply.vmmc);
+		if (volt < 0)
+			return volt;
+
+		min_uV = max(volt - 300000, 2700000);
+		max_uV = min(volt + 300000, 3600000);
+
+		/*
+		 * Due to a limitation in the current implementation of
+		 * regulator_set_voltage_triplet() which is taking the lowest
+		 * voltage possible if below the target, search for a suitable
+		 * voltage in two steps and try to stay close to vmmc
+		 * with a 0.3V tolerance at first.
+		 */
+		if (!mmc_regulator_set_voltage_if_supported(mmc->supply.vqmmc,
+						min_uV, volt, max_uV))
+			return 0;
+
+		return mmc_regulator_set_voltage_if_supported(mmc->supply.vqmmc,
+						2700000, volt, 3600000);
+	default:
+		return -EINVAL;
+	}
+}
+EXPORT_SYMBOL_GPL(mmc_regulator_set_vqmmc);
+
 #endif /* CONFIG_REGULATOR */
 
 int mmc_regulator_get_supply(struct mmc_host *mmc)
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 83b81fd..a2a78eb 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -423,6 +423,7 @@ int mmc_regulator_get_ocrmask(struct regulator *supply);
 int mmc_regulator_set_ocr(struct mmc_host *mmc,
 			struct regulator *supply,
 			unsigned short vdd_bit);
+int mmc_regulator_set_vqmmc(struct mmc_host *mmc, struct mmc_ios *ios);
 #else
 static inline int mmc_regulator_get_ocrmask(struct regulator *supply)
 {
@@ -435,6 +436,12 @@ static inline int mmc_regulator_set_ocr(struct mmc_host *mmc,
 {
 	return 0;
 }
+
+static inline int mmc_regulator_set_vqmmc(struct mmc_host *mmc,
+					  struct mmc_ios *ios)
+{
+	return -EINVAL;
+}
 #endif
 
 int mmc_regulator_get_supply(struct mmc_host *mmc);
-- 
2.5.1

[PATCH v2 3/8] mmc: core: Add mmc_regulator_set_vqmmc()

[Heiko Stuebner]

From: Douglas Anderson <dianders@chromium.org>

This adds logic to the MMC core to set VQMMC.  This is expected to be
called by MMC drivers like dw_mmc as part of (or instead of) their
start_signal_voltage_switch() callback.

A few notes:

* When setting the signal voltage to 3.3V we do our best to make VQMMC
  and VMMC match.  It's been reported that this makes some old cards
  happy since they were tested back in the day before UHS when VQMMC
  and VMMC were provided by the same regulator.  A nice side effect of
  this is that we don't end up on the hairy edge of VQMMC (2.7V),
  which some EEs claim is a little too close to the minimum for
  comfort.
  This is done in two steps. At first we try to find a VQMMC within
  a 0.3V tolerance of VMMC and if this is not supported by the
  supplying regulator we try to find a suitable voltage within the
  whole 2.7V-3.6V area of the spec.

* The two step approach is currently necessary, as the used
  regulator_set_voltage_triplet(min, target, max) uses a simple
  implementation that just tries two basic steps:
	regulator_set_voltage(target, max);
	regulator_set_voltage(min, target);
  So with only one step with 2.7-3.6V borders, if a suitable voltage
  is a bit below VMMC, we would directly get the lowest 2.7V
  which some boards (like Rockchips) don't like at all.

* When setting the signal voltage to 1.8V or 1.2V we aim for that
  specific voltage instead of picking the lowest one in the range.

* We very purposely don't print errors in mmc_regulator_set_vqmmc().
  There are cases where the MMC core will try several different
  voltages and we don't want to pollute the logs.

Signed-off-by: Douglas Anderson <dianders@chromium.org>
Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
 drivers/mmc/core/core.c  | 74 ++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/mmc/host.h |  7 +++++
 2 files changed, 81 insertions(+)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 0520064..363cc98 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -1437,6 +1437,80 @@ int mmc_regulator_set_ocr(struct mmc_host *mmc,
 }
 EXPORT_SYMBOL_GPL(mmc_regulator_set_ocr);
 
+static int mmc_regulator_set_voltage_if_supported(struct regulator *regulator,
+						  int min_uV, int target_uV,
+						  int max_uV)
+{
+	/*
+	 * Check if supported first to avoid errors since we may try several
+	 * signal levels during power up and don't want to show errors.
+	 */
+	if (!regulator_is_supported_voltage(regulator, min_uV, max_uV))
+		return -EINVAL;
+
+	return regulator_set_voltage_triplet(regulator, min_uV, target_uV,
+					     max_uV);
+}
+
+/**
+ * mmc_regulator_set_vqmmc - Set VQMMC as per the ios
+ *
+ * For 3.3V signaling, we try to match VQMMC to VMMC as closely as possible.
+ * That will match the behavior of old boards where VQMMC and VMMC were supplied
+ * by the same supply.  The Bus Operating conditions for 3.3V signaling in the
+ * SD card spec also define VQMMC in terms of VMMC.
+ * If this is not possible we'll try the full 2.7-3.6V of the spec.
+ *
+ * For 1.2V and 1.8V signaling we'll try to get as close as possible to the
+ * requested voltage.  This is definitely a good idea for UHS where there's a
+ * separate regulator on the card that's trying to make 1.8V and it's best if
+ * we match.
+ *
+ * This function is expected to be used by a controller's
+ * start_signal_voltage_switch() function.
+ */
+int mmc_regulator_set_vqmmc(struct mmc_host *mmc, struct mmc_ios *ios)
+{
+	int volt, min_uV, max_uV;
+
+	/* If no vqmmc supply then we can't change the voltage */
+	if (IS_ERR(mmc->supply.vqmmc))
+		return -EINVAL;
+
+	switch (ios->signal_voltage) {
+	case MMC_SIGNAL_VOLTAGE_120:
+		return mmc_regulator_set_voltage_if_supported(mmc->supply.vqmmc,
+						1100000, 1200000, 1300000);
+	case MMC_SIGNAL_VOLTAGE_180:
+		return mmc_regulator_set_voltage_if_supported(mmc->supply.vqmmc,
+						1700000, 1800000, 1950000);
+	case MMC_SIGNAL_VOLTAGE_330:
+		volt = regulator_get_voltage(mmc->supply.vmmc);
+		if (volt < 0)
+			return volt;
+
+		min_uV = max(volt - 300000, 2700000);
+		max_uV = min(volt + 300000, 3600000);
+
+		/*
+		 * Due to a limitation in the current implementation of
+		 * regulator_set_voltage_triplet() which is taking the lowest
+		 * voltage possible if below the target, search for a suitable
+		 * voltage in two steps and try to stay close to vmmc
+		 * with a 0.3V tolerance@first.
+		 */
+		if (!mmc_regulator_set_voltage_if_supported(mmc->supply.vqmmc,
+						min_uV, volt, max_uV))
+			return 0;
+
+		return mmc_regulator_set_voltage_if_supported(mmc->supply.vqmmc,
+						2700000, volt, 3600000);
+	default:
+		return -EINVAL;
+	}
+}
+EXPORT_SYMBOL_GPL(mmc_regulator_set_vqmmc);
+
 #endif /* CONFIG_REGULATOR */
 
 int mmc_regulator_get_supply(struct mmc_host *mmc)
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 83b81fd..a2a78eb 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -423,6 +423,7 @@ int mmc_regulator_get_ocrmask(struct regulator *supply);
 int mmc_regulator_set_ocr(struct mmc_host *mmc,
 			struct regulator *supply,
 			unsigned short vdd_bit);
+int mmc_regulator_set_vqmmc(struct mmc_host *mmc, struct mmc_ios *ios);
 #else
 static inline int mmc_regulator_get_ocrmask(struct regulator *supply)
 {
@@ -435,6 +436,12 @@ static inline int mmc_regulator_set_ocr(struct mmc_host *mmc,
 {
 	return 0;
 }
+
+static inline int mmc_regulator_set_vqmmc(struct mmc_host *mmc,
+					  struct mmc_ios *ios)
+{
+	return -EINVAL;
+}
 #endif
 
 int mmc_regulator_get_supply(struct mmc_host *mmc);
-- 
2.5.1

[PATCH v2 4/8] mmc: dw_mmc: Use mmc_regulator_set_vqmmc in start_signal_voltage_switch

[Heiko Stuebner]

From: Douglas Anderson <dianders@chromium.org>

We've introduced a new helper in the MMC core:
mmc_regulator_set_vqmmc().  Let's use this in dw_mmc.  Using this new
helper has some advantages:

1. We get the mmc_regulator_set_vqmmc() behavior of trying to match
   VQMMC and VMMC when the signal voltage is 3.3V.  This ensures max
   compatibility.

2. We get rid of a few more warnings when probing unsupported
   voltages.

3. We get rid of some non-dw_mmc specific code in dw_mmc.

Signed-off-by: Douglas Anderson <dianders@chromium.org>
Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
 drivers/mmc/host/dw_mmc.c | 17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index fcbf552..b1b7e7f 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -1279,7 +1279,6 @@ static int dw_mci_switch_voltage(struct mmc_host *mmc, struct mmc_ios *ios)
 	const struct dw_mci_drv_data *drv_data = host->drv_data;
 	u32 uhs;
 	u32 v18 = SDMMC_UHS_18V << slot->id;
-	int min_uv, max_uv;
 	int ret;
 
 	if (drv_data && drv_data->switch_voltage)
@@ -1291,22 +1290,18 @@ static int dw_mci_switch_voltage(struct mmc_host *mmc, struct mmc_ios *ios)
 	 * does no harm but you need to set the regulator directly.  Try both.
 	 */
 	uhs = mci_readl(host, UHS_REG);
-	if (ios->signal_voltage == MMC_SIGNAL_VOLTAGE_330) {
-		min_uv = 2700000;
-		max_uv = 3600000;
+	if (ios->signal_voltage == MMC_SIGNAL_VOLTAGE_330)
 		uhs &= ~v18;
-	} else {
-		min_uv = 1700000;
-		max_uv = 1950000;
+	else
 		uhs |= v18;
-	}
+
 	if (!IS_ERR(mmc->supply.vqmmc)) {
-		ret = regulator_set_voltage(mmc->supply.vqmmc, min_uv, max_uv);
+		ret = mmc_regulator_set_vqmmc(mmc, ios);
 
 		if (ret) {
 			dev_dbg(&mmc->class_dev,
-					 "Regulator set error %d: %d - %d\n",
-					 ret, min_uv, max_uv);
+					 "Regulator set error %d - %s V\n",
+					 ret, uhs & v18 ? "1.8" : "3.3");
 			return ret;
 		}
 	}
-- 
2.5.1

[PATCH v2 4/8] mmc: dw_mmc: Use mmc_regulator_set_vqmmc in start_signal_voltage_switch

[Heiko Stuebner]

From: Douglas Anderson <dianders@chromium.org>

We've introduced a new helper in the MMC core:
mmc_regulator_set_vqmmc().  Let's use this in dw_mmc.  Using this new
helper has some advantages:

1. We get the mmc_regulator_set_vqmmc() behavior of trying to match
   VQMMC and VMMC when the signal voltage is 3.3V.  This ensures max
   compatibility.

2. We get rid of a few more warnings when probing unsupported
   voltages.

3. We get rid of some non-dw_mmc specific code in dw_mmc.

Signed-off-by: Douglas Anderson <dianders@chromium.org>
Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
 drivers/mmc/host/dw_mmc.c | 17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index fcbf552..b1b7e7f 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -1279,7 +1279,6 @@ static int dw_mci_switch_voltage(struct mmc_host *mmc, struct mmc_ios *ios)
 	const struct dw_mci_drv_data *drv_data = host->drv_data;
 	u32 uhs;
 	u32 v18 = SDMMC_UHS_18V << slot->id;
-	int min_uv, max_uv;
 	int ret;
 
 	if (drv_data && drv_data->switch_voltage)
@@ -1291,22 +1290,18 @@ static int dw_mci_switch_voltage(struct mmc_host *mmc, struct mmc_ios *ios)
 	 * does no harm but you need to set the regulator directly.  Try both.
 	 */
 	uhs = mci_readl(host, UHS_REG);
-	if (ios->signal_voltage == MMC_SIGNAL_VOLTAGE_330) {
-		min_uv = 2700000;
-		max_uv = 3600000;
+	if (ios->signal_voltage == MMC_SIGNAL_VOLTAGE_330)
 		uhs &= ~v18;
-	} else {
-		min_uv = 1700000;
-		max_uv = 1950000;
+	else
 		uhs |= v18;
-	}
+
 	if (!IS_ERR(mmc->supply.vqmmc)) {
-		ret = regulator_set_voltage(mmc->supply.vqmmc, min_uv, max_uv);
+		ret = mmc_regulator_set_vqmmc(mmc, ios);
 
 		if (ret) {
 			dev_dbg(&mmc->class_dev,
-					 "Regulator set error %d: %d - %d\n",
-					 ret, min_uv, max_uv);
+					 "Regulator set error %d - %s V\n",
+					 ret, uhs & v18 ? "1.8" : "3.3");
 			return ret;
 		}
 	}
-- 
2.5.1

[PATCH v2 5/8] mmc: dw_mmc-rockchip: dt-binding: Add tuning related things

[Heiko Stuebner]

From: Alexandru M Stan <amstan@chromium.org>

Add ciu_drive, ciu_sample clocks and default-sample-phase. This will later
be used by tuning code.

We do not touch ciu_drive (and by extension define default-drive-phase).
Drive phase is mostly used to define minimum hold times, while one could
write some code to determine what phase meets the minimum hold time
(ex 10 degrees) this will not work with the current clock phase framework
(which floors angles, so we'll get 0 deg, and there's no way to know what
resolution the floors happen at). We assume that the default drive angles
set by the hardware are good enough.

Signed-off-by: Alexandru M Stan <amstan@chromium.org>
Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
 Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt b/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt
index c327c2d..3dc13b6 100644
--- a/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt
+++ b/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt
@@ -14,6 +14,19 @@ Required Properties:
 							before RK3288
 	- "rockchip,rk3288-dw-mshc": for Rockchip RK3288
 
+Optional Properties:
+* clocks: from common clock binding: if ciu_drive and ciu_sample are
+  specified in clock-names, should contain handles to these clocks.
+
+* clock-names: Apart from the clock-names described in synopsys-dw-mshc.txt
+  two more clocks "ciu-drive" and "ciu-sample" are supported. They are used
+  to control the clock phases, "ciu-sample" is required for tuning high-
+  speed modes.
+
+* rockchip,default-sample-phase: The default phase to set ciu_sample at
+  probing, low speeds or in case where all phases work at tuning time.
+  If not specified 0 deg will be used.
+
 Example:
 
 	rkdwmmc0@12200000 {
-- 
2.5.1

[PATCH v2 5/8] mmc: dw_mmc-rockchip: dt-binding: Add tuning related things

[Heiko Stuebner]

From: Alexandru M Stan <amstan@chromium.org>

Add ciu_drive, ciu_sample clocks and default-sample-phase. This will later
be used by tuning code.

We do not touch ciu_drive (and by extension define default-drive-phase).
Drive phase is mostly used to define minimum hold times, while one could
write some code to determine what phase meets the minimum hold time
(ex 10 degrees) this will not work with the current clock phase framework
(which floors angles, so we'll get 0 deg, and there's no way to know what
resolution the floors happen at). We assume that the default drive angles
set by the hardware are good enough.

Signed-off-by: Alexandru M Stan <amstan@chromium.org>
Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
 Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt b/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt
index c327c2d..3dc13b6 100644
--- a/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt
+++ b/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt
@@ -14,6 +14,19 @@ Required Properties:
 							before RK3288
 	- "rockchip,rk3288-dw-mshc": for Rockchip RK3288
 
+Optional Properties:
+* clocks: from common clock binding: if ciu_drive and ciu_sample are
+  specified in clock-names, should contain handles to these clocks.
+
+* clock-names: Apart from the clock-names described in synopsys-dw-mshc.txt
+  two more clocks "ciu-drive" and "ciu-sample" are supported. They are used
+  to control the clock phases, "ciu-sample" is required for tuning high-
+  speed modes.
+
+* rockchip,default-sample-phase: The default phase to set ciu_sample at
+  probing, low speeds or in case where all phases work at tuning time.
+  If not specified 0 deg will be used.
+
 Example:
 
 	rkdwmmc0 at 12200000 {
-- 
2.5.1

[PATCH v2 6/8] mmc: dw_mmc-rockchip: MMC tuning with the clock phase framework

[Heiko Stuebner]

From: Alexandru M Stan <amstan@chromium.org>

This algorithm will try 1 degree increments, since there's no way to tell
what resolution the underlying phase code uses. As an added bonus, doing
many tunings yields better results since some tests are run more than once
(ex: if the underlying driver uses 45 degree increments, the tuning code
will try the same angle more than once).

It will then construct a list of good phase ranges (even ranges that cross
360/0), will pick the biggest range then it will set the sample_clk to the
middle of that range.

We do not touch ciu_drive (and by extension define default-drive-phase).
Drive phase is mostly used to define minimum hold times, while one could
write some code to determine what phase meets the minimum hold time (ex 10
degrees) this will not work with the current clock phase framework (which
floors angles, so we'll get 0 deg, and there's no way to know what
resolution the floors happen at). We assume that the default drive angles
set by the hardware are good enough.

If a device has device specific code (like exynos) then that will still
take precedence, otherwise this new code will execute. If the device wants
to tune, but has no sample_clk defined we'll return EIO with an error
message.

Signed-off-by: Alexandru M Stan <amstan@chromium.org>
Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
 drivers/mmc/host/dw_mmc-rockchip.c | 162 +++++++++++++++++++++++++++++++++++++
 1 file changed, 162 insertions(+)

diff --git a/drivers/mmc/host/dw_mmc-rockchip.c b/drivers/mmc/host/dw_mmc-rockchip.c
index bc76aa2..4b3650f 100644
--- a/drivers/mmc/host/dw_mmc-rockchip.c
+++ b/drivers/mmc/host/dw_mmc-rockchip.c
@@ -13,12 +13,19 @@
 #include <linux/mmc/host.h>
 #include <linux/mmc/dw_mmc.h>
 #include <linux/of_address.h>
+#include <linux/slab.h>
 
 #include "dw_mmc.h"
 #include "dw_mmc-pltfm.h"
 
 #define RK3288_CLKGEN_DIV       2
 
+struct dw_mci_rockchip_priv_data {
+	struct clk		*drv_clk;
+	struct clk		*sample_clk;
+	int			default_sample_phase;
+};
+
 static void dw_mci_rockchip_prepare_command(struct dw_mci *host, u32 *cmdr)
 {
 	*cmdr |= SDMMC_CMD_USE_HOLD_REG;
@@ -33,6 +40,7 @@ static int dw_mci_rk3288_setup_clock(struct dw_mci *host)
 
 static void dw_mci_rk3288_set_ios(struct dw_mci *host, struct mmc_ios *ios)
 {
+	struct dw_mci_rockchip_priv_data *priv = host->priv;
 	int ret;
 	unsigned int cclkin;
 	u32 bus_hz;
@@ -66,6 +74,158 @@ static void dw_mci_rk3288_set_ios(struct dw_mci *host, struct mmc_ios *ios)
 		/* force dw_mci_setup_bus() */
 		host->current_speed = 0;
 	}
+
+	/* Make sure we use phases which we can enumerate with */
+	if (!IS_ERR(priv->sample_clk))
+		clk_set_phase(priv->sample_clk, priv->default_sample_phase);
+}
+
+#define NUM_PHASES			360
+#define TUNING_ITERATION_TO_PHASE(i)	(DIV_ROUND_UP((i) * 360, NUM_PHASES))
+
+static int dw_mci_rk3288_execute_tuning(struct dw_mci_slot *slot)
+{
+	struct dw_mci *host = slot->host;
+	struct dw_mci_rockchip_priv_data *priv = host->priv;
+	struct mmc_host *mmc = slot->mmc;
+	int ret = 0;
+	int i;
+	bool v, prev_v = 0, first_v;
+	struct range_t {
+		int start;
+		int end; /* inclusive */
+	};
+	struct range_t *ranges;
+	unsigned int range_count = 0;
+	int longest_range_len = -1;
+	int longest_range = -1;
+	int middle_phase;
+
+	if (IS_ERR(priv->sample_clk)) {
+		dev_err(host->dev, "Tuning clock (sample_clk) not defined.\n");
+		return -EIO;
+	}
+
+	ranges = kmalloc_array(NUM_PHASES / 2 + 1, sizeof(*ranges), GFP_KERNEL);
+	if (!ranges)
+		return -ENOMEM;
+
+	/* Try each phase and extract good ranges */
+	for (i = 0; i < NUM_PHASES; ) {
+		clk_set_phase(priv->sample_clk, TUNING_ITERATION_TO_PHASE(i));
+
+		v = !mmc_send_tuning(mmc);
+
+		if (i == 0)
+			first_v = v;
+
+		if ((!prev_v) && v) {
+			range_count++;
+			ranges[range_count-1].start = i;
+		}
+		if (v) {
+			ranges[range_count-1].end = i;
+			i++;
+		} else if (i == NUM_PHASES - 1) {
+			/* No extra skipping rules if we're at the end */
+			i++;
+		} else {
+			/*
+			 * No need to check too close to an invalid
+			 * one since testing bad phases is slow.  Skip
+			 * 20 degrees.
+			 */
+			i += DIV_ROUND_UP(20 * NUM_PHASES, 360);
+
+			/* Always test the last one */
+			if (i >= NUM_PHASES)
+				i = NUM_PHASES - 1;
+		}
+
+		prev_v = v;
+	}
+
+	if (range_count == 0) {
+		dev_warn(host->dev, "All phases bad!");
+		ret = -EIO;
+		goto free;
+	}
+
+	/* wrap around case, merge the end points */
+	if ((range_count > 1) && first_v && v) {
+		ranges[0].start = ranges[range_count-1].start;
+		range_count--;
+	}
+
+	if (ranges[0].start == 0 && ranges[0].end == NUM_PHASES - 1) {
+		clk_set_phase(priv->sample_clk, priv->default_sample_phase);
+		dev_info(host->dev, "All phases work, using default phase %d.",
+			 priv->default_sample_phase);
+		goto free;
+	}
+
+	/* Find the longest range */
+	for (i = 0; i < range_count; i++) {
+		int len = (ranges[i].end - ranges[i].start + 1);
+
+		if (len < 0)
+			len += NUM_PHASES;
+
+		if (longest_range_len < len) {
+			longest_range_len = len;
+			longest_range = i;
+		}
+
+		dev_dbg(host->dev, "Good phase range %d-%d (%d len)\n",
+			TUNING_ITERATION_TO_PHASE(ranges[i].start),
+			TUNING_ITERATION_TO_PHASE(ranges[i].end),
+			len
+		);
+	}
+
+	dev_dbg(host->dev, "Best phase range %d-%d (%d len)\n",
+		TUNING_ITERATION_TO_PHASE(ranges[longest_range].start),
+		TUNING_ITERATION_TO_PHASE(ranges[longest_range].end),
+		longest_range_len
+	);
+
+	middle_phase = ranges[longest_range].start + longest_range_len / 2;
+	middle_phase %= NUM_PHASES;
+	dev_info(host->dev, "Successfully tuned phase to %d\n",
+		 TUNING_ITERATION_TO_PHASE(middle_phase));
+
+	clk_set_phase(priv->sample_clk,
+		      TUNING_ITERATION_TO_PHASE(middle_phase));
+
+free:
+	kfree(ranges);
+	return ret;
+}
+
+static int dw_mci_rk3288_parse_dt(struct dw_mci *host)
+{
+	struct device_node *np = host->dev->of_node;
+	struct dw_mci_rockchip_priv_data *priv;
+
+	priv = devm_kzalloc(host->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	if (of_property_read_u32(np, "rockchip,default-sample-phase",
+					&priv->default_sample_phase))
+		priv->default_sample_phase = 0;
+
+	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");
+
+	priv->sample_clk = devm_clk_get(host->dev, "ciu-sample");
+	if (IS_ERR(priv->sample_clk))
+		dev_dbg(host->dev, "ciu_sample not available\n");
+
+	host->priv = priv;
+
+	return 0;
 }
 
 static int dw_mci_rockchip_init(struct dw_mci *host)
@@ -95,6 +255,8 @@ static const struct dw_mci_drv_data rk3288_drv_data = {
 	.caps			= dw_mci_rk3288_dwmmc_caps,
 	.prepare_command        = dw_mci_rockchip_prepare_command,
 	.set_ios		= dw_mci_rk3288_set_ios,
+	.execute_tuning		= dw_mci_rk3288_execute_tuning,
+	.parse_dt		= dw_mci_rk3288_parse_dt,
 	.setup_clock    = dw_mci_rk3288_setup_clock,
 	.init			= dw_mci_rockchip_init,
 };
-- 
2.5.1

[PATCH v2 6/8] mmc: dw_mmc-rockchip: MMC tuning with the clock phase framework

[Heiko Stuebner]

From: Alexandru M Stan <amstan@chromium.org>

This algorithm will try 1 degree increments, since there's no way to tell
what resolution the underlying phase code uses. As an added bonus, doing
many tunings yields better results since some tests are run more than once
(ex: if the underlying driver uses 45 degree increments, the tuning code
will try the same angle more than once).

It will then construct a list of good phase ranges (even ranges that cross
360/0), will pick the biggest range then it will set the sample_clk to the
middle of that range.

We do not touch ciu_drive (and by extension define default-drive-phase).
Drive phase is mostly used to define minimum hold times, while one could
write some code to determine what phase meets the minimum hold time (ex 10
degrees) this will not work with the current clock phase framework (which
floors angles, so we'll get 0 deg, and there's no way to know what
resolution the floors happen at). We assume that the default drive angles
set by the hardware are good enough.

If a device has device specific code (like exynos) then that will still
take precedence, otherwise this new code will execute. If the device wants
to tune, but has no sample_clk defined we'll return EIO with an error
message.

Signed-off-by: Alexandru M Stan <amstan@chromium.org>
Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
 drivers/mmc/host/dw_mmc-rockchip.c | 162 +++++++++++++++++++++++++++++++++++++
 1 file changed, 162 insertions(+)

diff --git a/drivers/mmc/host/dw_mmc-rockchip.c b/drivers/mmc/host/dw_mmc-rockchip.c
index bc76aa2..4b3650f 100644
--- a/drivers/mmc/host/dw_mmc-rockchip.c
+++ b/drivers/mmc/host/dw_mmc-rockchip.c
@@ -13,12 +13,19 @@
 #include <linux/mmc/host.h>
 #include <linux/mmc/dw_mmc.h>
 #include <linux/of_address.h>
+#include <linux/slab.h>
 
 #include "dw_mmc.h"
 #include "dw_mmc-pltfm.h"
 
 #define RK3288_CLKGEN_DIV       2
 
+struct dw_mci_rockchip_priv_data {
+	struct clk		*drv_clk;
+	struct clk		*sample_clk;
+	int			default_sample_phase;
+};
+
 static void dw_mci_rockchip_prepare_command(struct dw_mci *host, u32 *cmdr)
 {
 	*cmdr |= SDMMC_CMD_USE_HOLD_REG;
@@ -33,6 +40,7 @@ static int dw_mci_rk3288_setup_clock(struct dw_mci *host)
 
 static void dw_mci_rk3288_set_ios(struct dw_mci *host, struct mmc_ios *ios)
 {
+	struct dw_mci_rockchip_priv_data *priv = host->priv;
 	int ret;
 	unsigned int cclkin;
 	u32 bus_hz;
@@ -66,6 +74,158 @@ static void dw_mci_rk3288_set_ios(struct dw_mci *host, struct mmc_ios *ios)
 		/* force dw_mci_setup_bus() */
 		host->current_speed = 0;
 	}
+
+	/* Make sure we use phases which we can enumerate with */
+	if (!IS_ERR(priv->sample_clk))
+		clk_set_phase(priv->sample_clk, priv->default_sample_phase);
+}
+
+#define NUM_PHASES			360
+#define TUNING_ITERATION_TO_PHASE(i)	(DIV_ROUND_UP((i) * 360, NUM_PHASES))
+
+static int dw_mci_rk3288_execute_tuning(struct dw_mci_slot *slot)
+{
+	struct dw_mci *host = slot->host;
+	struct dw_mci_rockchip_priv_data *priv = host->priv;
+	struct mmc_host *mmc = slot->mmc;
+	int ret = 0;
+	int i;
+	bool v, prev_v = 0, first_v;
+	struct range_t {
+		int start;
+		int end; /* inclusive */
+	};
+	struct range_t *ranges;
+	unsigned int range_count = 0;
+	int longest_range_len = -1;
+	int longest_range = -1;
+	int middle_phase;
+
+	if (IS_ERR(priv->sample_clk)) {
+		dev_err(host->dev, "Tuning clock (sample_clk) not defined.\n");
+		return -EIO;
+	}
+
+	ranges = kmalloc_array(NUM_PHASES / 2 + 1, sizeof(*ranges), GFP_KERNEL);
+	if (!ranges)
+		return -ENOMEM;
+
+	/* Try each phase and extract good ranges */
+	for (i = 0; i < NUM_PHASES; ) {
+		clk_set_phase(priv->sample_clk, TUNING_ITERATION_TO_PHASE(i));
+
+		v = !mmc_send_tuning(mmc);
+
+		if (i == 0)
+			first_v = v;
+
+		if ((!prev_v) && v) {
+			range_count++;
+			ranges[range_count-1].start = i;
+		}
+		if (v) {
+			ranges[range_count-1].end = i;
+			i++;
+		} else if (i == NUM_PHASES - 1) {
+			/* No extra skipping rules if we're at the end */
+			i++;
+		} else {
+			/*
+			 * No need to check too close to an invalid
+			 * one since testing bad phases is slow.  Skip
+			 * 20 degrees.
+			 */
+			i += DIV_ROUND_UP(20 * NUM_PHASES, 360);
+
+			/* Always test the last one */
+			if (i >= NUM_PHASES)
+				i = NUM_PHASES - 1;
+		}
+
+		prev_v = v;
+	}
+
+	if (range_count == 0) {
+		dev_warn(host->dev, "All phases bad!");
+		ret = -EIO;
+		goto free;
+	}
+
+	/* wrap around case, merge the end points */
+	if ((range_count > 1) && first_v && v) {
+		ranges[0].start = ranges[range_count-1].start;
+		range_count--;
+	}
+
+	if (ranges[0].start == 0 && ranges[0].end == NUM_PHASES - 1) {
+		clk_set_phase(priv->sample_clk, priv->default_sample_phase);
+		dev_info(host->dev, "All phases work, using default phase %d.",
+			 priv->default_sample_phase);
+		goto free;
+	}
+
+	/* Find the longest range */
+	for (i = 0; i < range_count; i++) {
+		int len = (ranges[i].end - ranges[i].start + 1);
+
+		if (len < 0)
+			len += NUM_PHASES;
+
+		if (longest_range_len < len) {
+			longest_range_len = len;
+			longest_range = i;
+		}
+
+		dev_dbg(host->dev, "Good phase range %d-%d (%d len)\n",
+			TUNING_ITERATION_TO_PHASE(ranges[i].start),
+			TUNING_ITERATION_TO_PHASE(ranges[i].end),
+			len
+		);
+	}
+
+	dev_dbg(host->dev, "Best phase range %d-%d (%d len)\n",
+		TUNING_ITERATION_TO_PHASE(ranges[longest_range].start),
+		TUNING_ITERATION_TO_PHASE(ranges[longest_range].end),
+		longest_range_len
+	);
+
+	middle_phase = ranges[longest_range].start + longest_range_len / 2;
+	middle_phase %= NUM_PHASES;
+	dev_info(host->dev, "Successfully tuned phase to %d\n",
+		 TUNING_ITERATION_TO_PHASE(middle_phase));
+
+	clk_set_phase(priv->sample_clk,
+		      TUNING_ITERATION_TO_PHASE(middle_phase));
+
+free:
+	kfree(ranges);
+	return ret;
+}
+
+static int dw_mci_rk3288_parse_dt(struct dw_mci *host)
+{
+	struct device_node *np = host->dev->of_node;
+	struct dw_mci_rockchip_priv_data *priv;
+
+	priv = devm_kzalloc(host->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	if (of_property_read_u32(np, "rockchip,default-sample-phase",
+					&priv->default_sample_phase))
+		priv->default_sample_phase = 0;
+
+	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");
+
+	priv->sample_clk = devm_clk_get(host->dev, "ciu-sample");
+	if (IS_ERR(priv->sample_clk))
+		dev_dbg(host->dev, "ciu_sample not available\n");
+
+	host->priv = priv;
+
+	return 0;
 }
 
 static int dw_mci_rockchip_init(struct dw_mci *host)
@@ -95,6 +255,8 @@ static const struct dw_mci_drv_data rk3288_drv_data = {
 	.caps			= dw_mci_rk3288_dwmmc_caps,
 	.prepare_command        = dw_mci_rockchip_prepare_command,
 	.set_ios		= dw_mci_rk3288_set_ios,
+	.execute_tuning		= dw_mci_rk3288_execute_tuning,
+	.parse_dt		= dw_mci_rk3288_parse_dt,
 	.setup_clock    = dw_mci_rk3288_setup_clock,
 	.init			= dw_mci_rockchip_init,
 };
-- 
2.5.1

[PATCH v2 7/8] ARM: dts: rockchip: Add drive/sample clocks for rk3288 dw_mmc devices

[Heiko Stuebner]

From: Alexandru M Stan <amstan@chromium.org>

The drive/sample clocks can be phase shifted.  The drive clock
could be used in a future patch to adjust hold times.  The sample
clock is used for tuning.

Signed-off-by: Alexandru M Stan <amstan@chromium.org>
Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
 arch/arm/boot/dts/rk3288.dtsi | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/arch/arm/boot/dts/rk3288.dtsi b/arch/arm/boot/dts/rk3288.dtsi
index 27a397f6..37c5a02 100644
--- a/arch/arm/boot/dts/rk3288.dtsi
+++ b/arch/arm/boot/dts/rk3288.dtsi
@@ -225,8 +225,9 @@
 	sdmmc: dwmmc@ff0c0000 {
 		compatible = "rockchip,rk3288-dw-mshc";
 		clock-freq-min-max = <400000 150000000>;
-		clocks = <&cru HCLK_SDMMC>, <&cru SCLK_SDMMC>;
-		clock-names = "biu", "ciu";
+		clocks = <&cru HCLK_SDMMC>, <&cru SCLK_SDMMC>,
+			 <&cru SCLK_SDMMC_DRV>, <&cru SCLK_SDMMC_SAMPLE>;
+		clock-names = "biu", "ciu", "ciu-drive", "ciu-sample";
 		fifo-depth = <0x100>;
 		interrupts = <GIC_SPI 32 IRQ_TYPE_LEVEL_HIGH>;
 		reg = <0xff0c0000 0x4000>;
@@ -236,8 +237,9 @@
 	sdio0: dwmmc@ff0d0000 {
 		compatible = "rockchip,rk3288-dw-mshc";
 		clock-freq-min-max = <400000 150000000>;
-		clocks = <&cru HCLK_SDIO0>, <&cru SCLK_SDIO0>;
-		clock-names = "biu", "ciu";
+		clocks = <&cru HCLK_SDIO0>, <&cru SCLK_SDIO0>,
+			 <&cru SCLK_SDIO0_DRV>, <&cru SCLK_SDIO0_SAMPLE>;
+		clock-names = "biu", "ciu", "ciu-drive", "ciu-sample";
 		fifo-depth = <0x100>;
 		interrupts = <GIC_SPI 33 IRQ_TYPE_LEVEL_HIGH>;
 		reg = <0xff0d0000 0x4000>;
@@ -247,8 +249,9 @@
 	sdio1: dwmmc@ff0e0000 {
 		compatible = "rockchip,rk3288-dw-mshc";
 		clock-freq-min-max = <400000 150000000>;
-		clocks = <&cru HCLK_SDIO1>, <&cru SCLK_SDIO1>;
-		clock-names = "biu", "ciu";
+		clocks = <&cru HCLK_SDIO1>, <&cru SCLK_SDIO1>,
+			 <&cru SCLK_SDIO1_DRV>, <&cru SCLK_SDIO1_SAMPLE>;
+		clock-names = "biu", "ciu", "ciu-drive", "ciu-sample";
 		fifo-depth = <0x100>;
 		interrupts = <GIC_SPI 34 IRQ_TYPE_LEVEL_HIGH>;
 		reg = <0xff0e0000 0x4000>;
@@ -258,8 +261,9 @@
 	emmc: dwmmc@ff0f0000 {
 		compatible = "rockchip,rk3288-dw-mshc";
 		clock-freq-min-max = <400000 150000000>;
-		clocks = <&cru HCLK_EMMC>, <&cru SCLK_EMMC>;
-		clock-names = "biu", "ciu";
+		clocks = <&cru HCLK_EMMC>, <&cru SCLK_EMMC>,
+			 <&cru SCLK_EMMC_DRV>, <&cru SCLK_EMMC_SAMPLE>;
+		clock-names = "biu", "ciu", "ciu-drive", "ciu-sample";
 		fifo-depth = <0x100>;
 		interrupts = <GIC_SPI 35 IRQ_TYPE_LEVEL_HIGH>;
 		reg = <0xff0f0000 0x4000>;
-- 
2.5.1

[PATCH v2 7/8] ARM: dts: rockchip: Add drive/sample clocks for rk3288 dw_mmc devices

[Heiko Stuebner]

From: Alexandru M Stan <amstan@chromium.org>

The drive/sample clocks can be phase shifted.  The drive clock
could be used in a future patch to adjust hold times.  The sample
clock is used for tuning.

Signed-off-by: Alexandru M Stan <amstan@chromium.org>
Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
 arch/arm/boot/dts/rk3288.dtsi | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/arch/arm/boot/dts/rk3288.dtsi b/arch/arm/boot/dts/rk3288.dtsi
index 27a397f6..37c5a02 100644
--- a/arch/arm/boot/dts/rk3288.dtsi
+++ b/arch/arm/boot/dts/rk3288.dtsi
@@ -225,8 +225,9 @@
 	sdmmc: dwmmc at ff0c0000 {
 		compatible = "rockchip,rk3288-dw-mshc";
 		clock-freq-min-max = <400000 150000000>;
-		clocks = <&cru HCLK_SDMMC>, <&cru SCLK_SDMMC>;
-		clock-names = "biu", "ciu";
+		clocks = <&cru HCLK_SDMMC>, <&cru SCLK_SDMMC>,
+			 <&cru SCLK_SDMMC_DRV>, <&cru SCLK_SDMMC_SAMPLE>;
+		clock-names = "biu", "ciu", "ciu-drive", "ciu-sample";
 		fifo-depth = <0x100>;
 		interrupts = <GIC_SPI 32 IRQ_TYPE_LEVEL_HIGH>;
 		reg = <0xff0c0000 0x4000>;
@@ -236,8 +237,9 @@
 	sdio0: dwmmc at ff0d0000 {
 		compatible = "rockchip,rk3288-dw-mshc";
 		clock-freq-min-max = <400000 150000000>;
-		clocks = <&cru HCLK_SDIO0>, <&cru SCLK_SDIO0>;
-		clock-names = "biu", "ciu";
+		clocks = <&cru HCLK_SDIO0>, <&cru SCLK_SDIO0>,
+			 <&cru SCLK_SDIO0_DRV>, <&cru SCLK_SDIO0_SAMPLE>;
+		clock-names = "biu", "ciu", "ciu-drive", "ciu-sample";
 		fifo-depth = <0x100>;
 		interrupts = <GIC_SPI 33 IRQ_TYPE_LEVEL_HIGH>;
 		reg = <0xff0d0000 0x4000>;
@@ -247,8 +249,9 @@
 	sdio1: dwmmc at ff0e0000 {
 		compatible = "rockchip,rk3288-dw-mshc";
 		clock-freq-min-max = <400000 150000000>;
-		clocks = <&cru HCLK_SDIO1>, <&cru SCLK_SDIO1>;
-		clock-names = "biu", "ciu";
+		clocks = <&cru HCLK_SDIO1>, <&cru SCLK_SDIO1>,
+			 <&cru SCLK_SDIO1_DRV>, <&cru SCLK_SDIO1_SAMPLE>;
+		clock-names = "biu", "ciu", "ciu-drive", "ciu-sample";
 		fifo-depth = <0x100>;
 		interrupts = <GIC_SPI 34 IRQ_TYPE_LEVEL_HIGH>;
 		reg = <0xff0e0000 0x4000>;
@@ -258,8 +261,9 @@
 	emmc: dwmmc at ff0f0000 {
 		compatible = "rockchip,rk3288-dw-mshc";
 		clock-freq-min-max = <400000 150000000>;
-		clocks = <&cru HCLK_EMMC>, <&cru SCLK_EMMC>;
-		clock-names = "biu", "ciu";
+		clocks = <&cru HCLK_EMMC>, <&cru SCLK_EMMC>,
+			 <&cru SCLK_EMMC_DRV>, <&cru SCLK_EMMC_SAMPLE>;
+		clock-names = "biu", "ciu", "ciu-drive", "ciu-sample";
 		fifo-depth = <0x100>;
 		interrupts = <GIC_SPI 35 IRQ_TYPE_LEVEL_HIGH>;
 		reg = <0xff0f0000 0x4000>;
-- 
2.5.1

[PATCH v2 8/8] ARM: dts: rockchip: add tuning related settings to veyron devices

[Heiko Stuebner]

This allows the tuning code to run and use higher speeds on capable cards.

Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
 arch/arm/boot/dts/rk3288-veyron-sdmmc.dtsi | 7 ++++++-
 arch/arm/boot/dts/rk3288-veyron.dtsi       | 6 ++++++
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/rk3288-veyron-sdmmc.dtsi b/arch/arm/boot/dts/rk3288-veyron-sdmmc.dtsi
index b5334ec..24a7a11 100644
--- a/arch/arm/boot/dts/rk3288-veyron-sdmmc.dtsi
+++ b/arch/arm/boot/dts/rk3288-veyron-sdmmc.dtsi
@@ -90,7 +90,7 @@
 	regulators {
 		vccio_sd: LDO_REG4 {
 			regulator-name = "vccio_sd";
-			regulator-min-microvolt = <3300000>;
+			regulator-min-microvolt = <1800000>;
 			regulator-max-microvolt = <3300000>;
 			regulator-state-mem {
 				regulator-off-in-suspend;
@@ -116,7 +116,12 @@
 	cap-sd-highspeed;
 	card-detect-delay = <200>;
 	cd-gpios = <&gpio7 5 GPIO_ACTIVE_LOW>;
+	rockchip,default-sample-phase = <90>;
 	num-slots = <1>;
+	sd-uhs-sdr12;
+	sd-uhs-sdr25;
+	sd-uhs-sdr50;
+	sd-uhs-sdr104;
 	vmmc-supply = <&vcc33_sd>;
 	vqmmc-supply = <&vccio_sd>;
 };
diff --git a/arch/arm/boot/dts/rk3288-veyron.dtsi b/arch/arm/boot/dts/rk3288-veyron.dtsi
index 6820977..aabd394 100644
--- a/arch/arm/boot/dts/rk3288-veyron.dtsi
+++ b/arch/arm/boot/dts/rk3288-veyron.dtsi
@@ -196,7 +196,9 @@
 	broken-cd;
 	bus-width = <8>;
 	cap-mmc-highspeed;
+	rockchip,default-sample-phase = <158>;
 	disable-wp;
+	mmc-hs200-1_8v;
 	mmc-pwrseq = <&emmc_pwrseq>;
 	non-removable;
 	num-slots = <1>;
@@ -413,6 +415,10 @@
 	num-slots = <1>;
 	pinctrl-names = "default";
 	pinctrl-0 = <&sdio0_clk &sdio0_cmd &sdio0_bus4>;
+	sd-uhs-sdr12;
+	sd-uhs-sdr25;
+	sd-uhs-sdr50;
+	sd-uhs-sdr104;
 	vmmc-supply = <&vcc33_sys>;
 	vqmmc-supply = <&vcc18_wl>;
 };
-- 
2.5.1

[PATCH v2 8/8] ARM: dts: rockchip: add tuning related settings to veyron devices

[Heiko Stuebner]

This allows the tuning code to run and use higher speeds on capable cards.

Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
 arch/arm/boot/dts/rk3288-veyron-sdmmc.dtsi | 7 ++++++-
 arch/arm/boot/dts/rk3288-veyron.dtsi       | 6 ++++++
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/rk3288-veyron-sdmmc.dtsi b/arch/arm/boot/dts/rk3288-veyron-sdmmc.dtsi
index b5334ec..24a7a11 100644
--- a/arch/arm/boot/dts/rk3288-veyron-sdmmc.dtsi
+++ b/arch/arm/boot/dts/rk3288-veyron-sdmmc.dtsi
@@ -90,7 +90,7 @@
 	regulators {
 		vccio_sd: LDO_REG4 {
 			regulator-name = "vccio_sd";
-			regulator-min-microvolt = <3300000>;
+			regulator-min-microvolt = <1800000>;
 			regulator-max-microvolt = <3300000>;
 			regulator-state-mem {
 				regulator-off-in-suspend;
@@ -116,7 +116,12 @@
 	cap-sd-highspeed;
 	card-detect-delay = <200>;
 	cd-gpios = <&gpio7 5 GPIO_ACTIVE_LOW>;
+	rockchip,default-sample-phase = <90>;
 	num-slots = <1>;
+	sd-uhs-sdr12;
+	sd-uhs-sdr25;
+	sd-uhs-sdr50;
+	sd-uhs-sdr104;
 	vmmc-supply = <&vcc33_sd>;
 	vqmmc-supply = <&vccio_sd>;
 };
diff --git a/arch/arm/boot/dts/rk3288-veyron.dtsi b/arch/arm/boot/dts/rk3288-veyron.dtsi
index 6820977..aabd394 100644
--- a/arch/arm/boot/dts/rk3288-veyron.dtsi
+++ b/arch/arm/boot/dts/rk3288-veyron.dtsi
@@ -196,7 +196,9 @@
 	broken-cd;
 	bus-width = <8>;
 	cap-mmc-highspeed;
+	rockchip,default-sample-phase = <158>;
 	disable-wp;
+	mmc-hs200-1_8v;
 	mmc-pwrseq = <&emmc_pwrseq>;
 	non-removable;
 	num-slots = <1>;
@@ -413,6 +415,10 @@
 	num-slots = <1>;
 	pinctrl-names = "default";
 	pinctrl-0 = <&sdio0_clk &sdio0_cmd &sdio0_bus4>;
+	sd-uhs-sdr12;
+	sd-uhs-sdr25;
+	sd-uhs-sdr50;
+	sd-uhs-sdr104;
 	vmmc-supply = <&vcc33_sys>;
 	vqmmc-supply = <&vcc18_wl>;
 };
-- 
2.5.1

Re: [PATCH v2 3/8] mmc: core: Add mmc_regulator_set_vqmmc()

[Ulf Hansson]

On 30 September 2015 at 16:07, Heiko Stuebner <heiko@sntech.de> wrote:
> From: Douglas Anderson <dianders@chromium.org>
>
> This adds logic to the MMC core to set VQMMC.  This is expected to be
> called by MMC drivers like dw_mmc as part of (or instead of) their
> start_signal_voltage_switch() callback.
>
> A few notes:
>
> * When setting the signal voltage to 3.3V we do our best to make VQMMC
>   and VMMC match.  It's been reported that this makes some old cards
>   happy since they were tested back in the day before UHS when VQMMC
>   and VMMC were provided by the same regulator.  A nice side effect of
>   this is that we don't end up on the hairy edge of VQMMC (2.7V),
>   which some EEs claim is a little too close to the minimum for
>   comfort.
>   This is done in two steps. At first we try to find a VQMMC within
>   a 0.3V tolerance of VMMC and if this is not supported by the
>   supplying regulator we try to find a suitable voltage within the
>   whole 2.7V-3.6V area of the spec.
>
> * The two step approach is currently necessary, as the used
>   regulator_set_voltage_triplet(min, target, max) uses a simple
>   implementation that just tries two basic steps:
>         regulator_set_voltage(target, max);
>         regulator_set_voltage(min, target);
>   So with only one step with 2.7-3.6V borders, if a suitable voltage
>   is a bit below VMMC, we would directly get the lowest 2.7V
>   which some boards (like Rockchips) don't like at all.
>
> * When setting the signal voltage to 1.8V or 1.2V we aim for that
>   specific voltage instead of picking the lowest one in the range.
>
> * We very purposely don't print errors in mmc_regulator_set_vqmmc().
>   There are cases where the MMC core will try several different
>   voltages and we don't want to pollute the logs.
>
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>

This looks good to me!

Once all are happy with the patches, can we take the mmc patches via
my mmc tree or does it all have to go together?

Kind regards
Uffe

> ---
>  drivers/mmc/core/core.c  | 74 ++++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/mmc/host.h |  7 +++++
>  2 files changed, 81 insertions(+)
>
> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> index 0520064..363cc98 100644
> --- a/drivers/mmc/core/core.c
> +++ b/drivers/mmc/core/core.c
> @@ -1437,6 +1437,80 @@ int mmc_regulator_set_ocr(struct mmc_host *mmc,
>  }
>  EXPORT_SYMBOL_GPL(mmc_regulator_set_ocr);
>
> +static int mmc_regulator_set_voltage_if_supported(struct regulator *regulator,
> +                                                 int min_uV, int target_uV,
> +                                                 int max_uV)
> +{
> +       /*
> +        * Check if supported first to avoid errors since we may try several
> +        * signal levels during power up and don't want to show errors.
> +        */
> +       if (!regulator_is_supported_voltage(regulator, min_uV, max_uV))
> +               return -EINVAL;
> +
> +       return regulator_set_voltage_triplet(regulator, min_uV, target_uV,
> +                                            max_uV);
> +}
> +
> +/**
> + * mmc_regulator_set_vqmmc - Set VQMMC as per the ios
> + *
> + * For 3.3V signaling, we try to match VQMMC to VMMC as closely as possible.
> + * That will match the behavior of old boards where VQMMC and VMMC were supplied
> + * by the same supply.  The Bus Operating conditions for 3.3V signaling in the
> + * SD card spec also define VQMMC in terms of VMMC.
> + * If this is not possible we'll try the full 2.7-3.6V of the spec.
> + *
> + * For 1.2V and 1.8V signaling we'll try to get as close as possible to the
> + * requested voltage.  This is definitely a good idea for UHS where there's a
> + * separate regulator on the card that's trying to make 1.8V and it's best if
> + * we match.
> + *
> + * This function is expected to be used by a controller's
> + * start_signal_voltage_switch() function.
> + */
> +int mmc_regulator_set_vqmmc(struct mmc_host *mmc, struct mmc_ios *ios)
> +{
> +       int volt, min_uV, max_uV;
> +
> +       /* If no vqmmc supply then we can't change the voltage */
> +       if (IS_ERR(mmc->supply.vqmmc))
> +               return -EINVAL;
> +
> +       switch (ios->signal_voltage) {
> +       case MMC_SIGNAL_VOLTAGE_120:
> +               return mmc_regulator_set_voltage_if_supported(mmc->supply.vqmmc,
> +                                               1100000, 1200000, 1300000);
> +       case MMC_SIGNAL_VOLTAGE_180:
> +               return mmc_regulator_set_voltage_if_supported(mmc->supply.vqmmc,
> +                                               1700000, 1800000, 1950000);
> +       case MMC_SIGNAL_VOLTAGE_330:
> +               volt = regulator_get_voltage(mmc->supply.vmmc);
> +               if (volt < 0)
> +                       return volt;
> +
> +               min_uV = max(volt - 300000, 2700000);
> +               max_uV = min(volt + 300000, 3600000);
> +
> +               /*
> +                * Due to a limitation in the current implementation of
> +                * regulator_set_voltage_triplet() which is taking the lowest
> +                * voltage possible if below the target, search for a suitable
> +                * voltage in two steps and try to stay close to vmmc
> +                * with a 0.3V tolerance at first.
> +                */
> +               if (!mmc_regulator_set_voltage_if_supported(mmc->supply.vqmmc,
> +                                               min_uV, volt, max_uV))
> +                       return 0;
> +
> +               return mmc_regulator_set_voltage_if_supported(mmc->supply.vqmmc,
> +                                               2700000, volt, 3600000);
> +       default:
> +               return -EINVAL;
> +       }
> +}
> +EXPORT_SYMBOL_GPL(mmc_regulator_set_vqmmc);
> +
>  #endif /* CONFIG_REGULATOR */
>
>  int mmc_regulator_get_supply(struct mmc_host *mmc)
> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> index 83b81fd..a2a78eb 100644
> --- a/include/linux/mmc/host.h
> +++ b/include/linux/mmc/host.h
> @@ -423,6 +423,7 @@ int mmc_regulator_get_ocrmask(struct regulator *supply);
>  int mmc_regulator_set_ocr(struct mmc_host *mmc,
>                         struct regulator *supply,
>                         unsigned short vdd_bit);
> +int mmc_regulator_set_vqmmc(struct mmc_host *mmc, struct mmc_ios *ios);
>  #else
>  static inline int mmc_regulator_get_ocrmask(struct regulator *supply)
>  {
> @@ -435,6 +436,12 @@ static inline int mmc_regulator_set_ocr(struct mmc_host *mmc,
>  {
>         return 0;
>  }
> +
> +static inline int mmc_regulator_set_vqmmc(struct mmc_host *mmc,
> +                                         struct mmc_ios *ios)
> +{
> +       return -EINVAL;
> +}
>  #endif
>
>  int mmc_regulator_get_supply(struct mmc_host *mmc);
> --
> 2.5.1
>

[PATCH v2 3/8] mmc: core: Add mmc_regulator_set_vqmmc()

[Ulf Hansson]

On 30 September 2015 at 16:07, Heiko Stuebner <heiko@sntech.de> wrote:
> From: Douglas Anderson <dianders@chromium.org>
>
> This adds logic to the MMC core to set VQMMC.  This is expected to be
> called by MMC drivers like dw_mmc as part of (or instead of) their
> start_signal_voltage_switch() callback.
>
> A few notes:
>
> * When setting the signal voltage to 3.3V we do our best to make VQMMC
>   and VMMC match.  It's been reported that this makes some old cards
>   happy since they were tested back in the day before UHS when VQMMC
>   and VMMC were provided by the same regulator.  A nice side effect of
>   this is that we don't end up on the hairy edge of VQMMC (2.7V),
>   which some EEs claim is a little too close to the minimum for
>   comfort.
>   This is done in two steps. At first we try to find a VQMMC within
>   a 0.3V tolerance of VMMC and if this is not supported by the
>   supplying regulator we try to find a suitable voltage within the
>   whole 2.7V-3.6V area of the spec.
>
> * The two step approach is currently necessary, as the used
>   regulator_set_voltage_triplet(min, target, max) uses a simple
>   implementation that just tries two basic steps:
>         regulator_set_voltage(target, max);
>         regulator_set_voltage(min, target);
>   So with only one step with 2.7-3.6V borders, if a suitable voltage
>   is a bit below VMMC, we would directly get the lowest 2.7V
>   which some boards (like Rockchips) don't like at all.
>
> * When setting the signal voltage to 1.8V or 1.2V we aim for that
>   specific voltage instead of picking the lowest one in the range.
>
> * We very purposely don't print errors in mmc_regulator_set_vqmmc().
>   There are cases where the MMC core will try several different
>   voltages and we don't want to pollute the logs.
>
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>

This looks good to me!

Once all are happy with the patches, can we take the mmc patches via
my mmc tree or does it all have to go together?

Kind regards
Uffe

> ---
>  drivers/mmc/core/core.c  | 74 ++++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/mmc/host.h |  7 +++++
>  2 files changed, 81 insertions(+)
>
> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> index 0520064..363cc98 100644
> --- a/drivers/mmc/core/core.c
> +++ b/drivers/mmc/core/core.c
> @@ -1437,6 +1437,80 @@ int mmc_regulator_set_ocr(struct mmc_host *mmc,
>  }
>  EXPORT_SYMBOL_GPL(mmc_regulator_set_ocr);
>
> +static int mmc_regulator_set_voltage_if_supported(struct regulator *regulator,
> +                                                 int min_uV, int target_uV,
> +                                                 int max_uV)
> +{
> +       /*
> +        * Check if supported first to avoid errors since we may try several
> +        * signal levels during power up and don't want to show errors.
> +        */
> +       if (!regulator_is_supported_voltage(regulator, min_uV, max_uV))
> +               return -EINVAL;
> +
> +       return regulator_set_voltage_triplet(regulator, min_uV, target_uV,
> +                                            max_uV);
> +}
> +
> +/**
> + * mmc_regulator_set_vqmmc - Set VQMMC as per the ios
> + *
> + * For 3.3V signaling, we try to match VQMMC to VMMC as closely as possible.
> + * That will match the behavior of old boards where VQMMC and VMMC were supplied
> + * by the same supply.  The Bus Operating conditions for 3.3V signaling in the
> + * SD card spec also define VQMMC in terms of VMMC.
> + * If this is not possible we'll try the full 2.7-3.6V of the spec.
> + *
> + * For 1.2V and 1.8V signaling we'll try to get as close as possible to the
> + * requested voltage.  This is definitely a good idea for UHS where there's a
> + * separate regulator on the card that's trying to make 1.8V and it's best if
> + * we match.
> + *
> + * This function is expected to be used by a controller's
> + * start_signal_voltage_switch() function.
> + */
> +int mmc_regulator_set_vqmmc(struct mmc_host *mmc, struct mmc_ios *ios)
> +{
> +       int volt, min_uV, max_uV;
> +
> +       /* If no vqmmc supply then we can't change the voltage */
> +       if (IS_ERR(mmc->supply.vqmmc))
> +               return -EINVAL;
> +
> +       switch (ios->signal_voltage) {
> +       case MMC_SIGNAL_VOLTAGE_120:
> +               return mmc_regulator_set_voltage_if_supported(mmc->supply.vqmmc,
> +                                               1100000, 1200000, 1300000);
> +       case MMC_SIGNAL_VOLTAGE_180:
> +               return mmc_regulator_set_voltage_if_supported(mmc->supply.vqmmc,
> +                                               1700000, 1800000, 1950000);
> +       case MMC_SIGNAL_VOLTAGE_330:
> +               volt = regulator_get_voltage(mmc->supply.vmmc);
> +               if (volt < 0)
> +                       return volt;
> +
> +               min_uV = max(volt - 300000, 2700000);
> +               max_uV = min(volt + 300000, 3600000);
> +
> +               /*
> +                * Due to a limitation in the current implementation of
> +                * regulator_set_voltage_triplet() which is taking the lowest
> +                * voltage possible if below the target, search for a suitable
> +                * voltage in two steps and try to stay close to vmmc
> +                * with a 0.3V tolerance at first.
> +                */
> +               if (!mmc_regulator_set_voltage_if_supported(mmc->supply.vqmmc,
> +                                               min_uV, volt, max_uV))
> +                       return 0;
> +
> +               return mmc_regulator_set_voltage_if_supported(mmc->supply.vqmmc,
> +                                               2700000, volt, 3600000);
> +       default:
> +               return -EINVAL;
> +       }
> +}
> +EXPORT_SYMBOL_GPL(mmc_regulator_set_vqmmc);
> +
>  #endif /* CONFIG_REGULATOR */
>
>  int mmc_regulator_get_supply(struct mmc_host *mmc)
> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> index 83b81fd..a2a78eb 100644
> --- a/include/linux/mmc/host.h
> +++ b/include/linux/mmc/host.h
> @@ -423,6 +423,7 @@ int mmc_regulator_get_ocrmask(struct regulator *supply);
>  int mmc_regulator_set_ocr(struct mmc_host *mmc,
>                         struct regulator *supply,
>                         unsigned short vdd_bit);
> +int mmc_regulator_set_vqmmc(struct mmc_host *mmc, struct mmc_ios *ios);
>  #else
>  static inline int mmc_regulator_get_ocrmask(struct regulator *supply)
>  {
> @@ -435,6 +436,12 @@ static inline int mmc_regulator_set_ocr(struct mmc_host *mmc,
>  {
>         return 0;
>  }
> +
> +static inline int mmc_regulator_set_vqmmc(struct mmc_host *mmc,
> +                                         struct mmc_ios *ios)
> +{
> +       return -EINVAL;
> +}
>  #endif
>
>  int mmc_regulator_get_supply(struct mmc_host *mmc);
> --
> 2.5.1
>

Re: [PATCH v2 3/8] mmc: core: Add mmc_regulator_set_vqmmc()

[Heiko Stübner]

Am Mittwoch, 30. September 2015, 16:42:05 schrieb Ulf Hansson:
> On 30 September 2015 at 16:07, Heiko Stuebner <heiko@sntech.de> wrote:
> > From: Douglas Anderson <dianders@chromium.org>
> > 
> > This adds logic to the MMC core to set VQMMC.  This is expected to be
> > called by MMC drivers like dw_mmc as part of (or instead of) their
> > start_signal_voltage_switch() callback.
> > 
> > A few notes:
> > 
> > * When setting the signal voltage to 3.3V we do our best to make VQMMC
> > 
> >   and VMMC match.  It's been reported that this makes some old cards
> >   happy since they were tested back in the day before UHS when VQMMC
> >   and VMMC were provided by the same regulator.  A nice side effect of
> >   this is that we don't end up on the hairy edge of VQMMC (2.7V),
> >   which some EEs claim is a little too close to the minimum for
> >   comfort.
> >   This is done in two steps. At first we try to find a VQMMC within
> >   a 0.3V tolerance of VMMC and if this is not supported by the
> >   supplying regulator we try to find a suitable voltage within the
> >   whole 2.7V-3.6V area of the spec.
> > 
> > * The two step approach is currently necessary, as the used
> > 
> >   regulator_set_voltage_triplet(min, target, max) uses a simple
> >   
> >   implementation that just tries two basic steps:
> >         regulator_set_voltage(target, max);
> >         regulator_set_voltage(min, target);
> >   
> >   So with only one step with 2.7-3.6V borders, if a suitable voltage
> >   is a bit below VMMC, we would directly get the lowest 2.7V
> >   which some boards (like Rockchips) don't like at all.
> > 
> > * When setting the signal voltage to 1.8V or 1.2V we aim for that
> > 
> >   specific voltage instead of picking the lowest one in the range.
> > 
> > * We very purposely don't print errors in mmc_regulator_set_vqmmc().
> > 
> >   There are cases where the MMC core will try several different
> >   voltages and we don't want to pollute the logs.
> > 
> > Signed-off-by: Douglas Anderson <dianders@chromium.org>
> > Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> 
> This looks good to me!

very cool :-)


> Once all are happy with the patches, can we take the mmc patches via
> my mmc tree or does it all have to go together?

The clock changes of course only touch internals of the phase-clocks, so 
should have no problem going through another tree.

For the devicetree part I'm unsure. If the boards enable the tuning-related 
settings without the new voltage handling, 2.7V gets set on all Rockchip 
boards which doesn't work on those at all.

So either the dts patches would need to go into your tree, I would need a 
stable branch or we could simply postpone dts changes for the next cycle.


Heiko

> > ---
> > 
> >  drivers/mmc/core/core.c  | 74
> >  ++++++++++++++++++++++++++++++++++++++++++++++++
> >  include/linux/mmc/host.h |  7 +++++
> >  2 files changed, 81 insertions(+)
> > 
> > diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> > index 0520064..363cc98 100644
> > --- a/drivers/mmc/core/core.c
> > +++ b/drivers/mmc/core/core.c
> > @@ -1437,6 +1437,80 @@ int mmc_regulator_set_ocr(struct mmc_host *mmc,
> > 
> >  }
> >  EXPORT_SYMBOL_GPL(mmc_regulator_set_ocr);
> > 
> > +static int mmc_regulator_set_voltage_if_supported(struct regulator
> > *regulator, +                                                 int min_uV,
> > int target_uV, +                                                 int
> > max_uV)
> > +{
> > +       /*
> > +        * Check if supported first to avoid errors since we may try
> > several +        * signal levels during power up and don't want to show
> > errors. +        */
> > +       if (!regulator_is_supported_voltage(regulator, min_uV, max_uV))
> > +               return -EINVAL;
> > +
> > +       return regulator_set_voltage_triplet(regulator, min_uV, target_uV,
> > +                                            max_uV);
> > +}
> > +
> > +/**
> > + * mmc_regulator_set_vqmmc - Set VQMMC as per the ios
> > + *
> > + * For 3.3V signaling, we try to match VQMMC to VMMC as closely as
> > possible. + * That will match the behavior of old boards where VQMMC and
> > VMMC were supplied + * by the same supply.  The Bus Operating conditions
> > for 3.3V signaling in the + * SD card spec also define VQMMC in terms of
> > VMMC.
> > + * If this is not possible we'll try the full 2.7-3.6V of the spec.
> > + *
> > + * For 1.2V and 1.8V signaling we'll try to get as close as possible to
> > the + * requested voltage.  This is definitely a good idea for UHS where
> > there's a + * separate regulator on the card that's trying to make 1.8V
> > and it's best if + * we match.
> > + *
> > + * This function is expected to be used by a controller's
> > + * start_signal_voltage_switch() function.
> > + */
> > +int mmc_regulator_set_vqmmc(struct mmc_host *mmc, struct mmc_ios *ios)
> > +{
> > +       int volt, min_uV, max_uV;
> > +
> > +       /* If no vqmmc supply then we can't change the voltage */
> > +       if (IS_ERR(mmc->supply.vqmmc))
> > +               return -EINVAL;
> > +
> > +       switch (ios->signal_voltage) {
> > +       case MMC_SIGNAL_VOLTAGE_120:
> > +               return
> > mmc_regulator_set_voltage_if_supported(mmc->supply.vqmmc, +              
> >                                 1100000, 1200000, 1300000); +       case
> > MMC_SIGNAL_VOLTAGE_180:
> > +               return
> > mmc_regulator_set_voltage_if_supported(mmc->supply.vqmmc, +              
> >                                 1700000, 1800000, 1950000); +       case
> > MMC_SIGNAL_VOLTAGE_330:
> > +               volt = regulator_get_voltage(mmc->supply.vmmc);
> > +               if (volt < 0)
> > +                       return volt;
> > +
> > +               min_uV = max(volt - 300000, 2700000);
> > +               max_uV = min(volt + 300000, 3600000);
> > +
> > +               /*
> > +                * Due to a limitation in the current implementation of
> > +                * regulator_set_voltage_triplet() which is taking the
> > lowest +                * voltage possible if below the target, search
> > for a suitable +                * voltage in two steps and try to stay
> > close to vmmc +                * with a 0.3V tolerance at first.
> > +                */
> > +               if
> > (!mmc_regulator_set_voltage_if_supported(mmc->supply.vqmmc, +            
> >                                   min_uV, volt, max_uV)) +               
> >        return 0;
> > +
> > +               return
> > mmc_regulator_set_voltage_if_supported(mmc->supply.vqmmc, +              
> >                                 2700000, volt, 3600000); +       default:
> > +               return -EINVAL;
> > +       }
> > +}
> > +EXPORT_SYMBOL_GPL(mmc_regulator_set_vqmmc);
> > +
> > 
> >  #endif /* CONFIG_REGULATOR */
> >  
> >  int mmc_regulator_get_supply(struct mmc_host *mmc)
> > 
> > diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> > index 83b81fd..a2a78eb 100644
> > --- a/include/linux/mmc/host.h
> > +++ b/include/linux/mmc/host.h
> > @@ -423,6 +423,7 @@ int mmc_regulator_get_ocrmask(struct regulator
> > *supply);> 
> >  int mmc_regulator_set_ocr(struct mmc_host *mmc,
> >  
> >                         struct regulator *supply,
> >                         unsigned short vdd_bit);
> > 
> > +int mmc_regulator_set_vqmmc(struct mmc_host *mmc, struct mmc_ios *ios);
> > 
> >  #else
> >  static inline int mmc_regulator_get_ocrmask(struct regulator *supply)
> >  {
> > 
> > @@ -435,6 +436,12 @@ static inline int mmc_regulator_set_ocr(struct
> > mmc_host *mmc,> 
> >  {
> >  
> >         return 0;
> >  
> >  }
> > 
> > +
> > +static inline int mmc_regulator_set_vqmmc(struct mmc_host *mmc,
> > +                                         struct mmc_ios *ios)
> > +{
> > +       return -EINVAL;
> > +}
> > 
> >  #endif
> >  
> >  int mmc_regulator_get_supply(struct mmc_host *mmc);
> > 
> > --
> > 2.5.1

[PATCH v2 3/8] mmc: core: Add mmc_regulator_set_vqmmc()

[Heiko Stübner]

Am Mittwoch, 30. September 2015, 16:42:05 schrieb Ulf Hansson:
> On 30 September 2015 at 16:07, Heiko Stuebner <heiko@sntech.de> wrote:
> > From: Douglas Anderson <dianders@chromium.org>
> > 
> > This adds logic to the MMC core to set VQMMC.  This is expected to be
> > called by MMC drivers like dw_mmc as part of (or instead of) their
> > start_signal_voltage_switch() callback.
> > 
> > A few notes:
> > 
> > * When setting the signal voltage to 3.3V we do our best to make VQMMC
> > 
> >   and VMMC match.  It's been reported that this makes some old cards
> >   happy since they were tested back in the day before UHS when VQMMC
> >   and VMMC were provided by the same regulator.  A nice side effect of
> >   this is that we don't end up on the hairy edge of VQMMC (2.7V),
> >   which some EEs claim is a little too close to the minimum for
> >   comfort.
> >   This is done in two steps. At first we try to find a VQMMC within
> >   a 0.3V tolerance of VMMC and if this is not supported by the
> >   supplying regulator we try to find a suitable voltage within the
> >   whole 2.7V-3.6V area of the spec.
> > 
> > * The two step approach is currently necessary, as the used
> > 
> >   regulator_set_voltage_triplet(min, target, max) uses a simple
> >   
> >   implementation that just tries two basic steps:
> >         regulator_set_voltage(target, max);
> >         regulator_set_voltage(min, target);
> >   
> >   So with only one step with 2.7-3.6V borders, if a suitable voltage
> >   is a bit below VMMC, we would directly get the lowest 2.7V
> >   which some boards (like Rockchips) don't like at all.
> > 
> > * When setting the signal voltage to 1.8V or 1.2V we aim for that
> > 
> >   specific voltage instead of picking the lowest one in the range.
> > 
> > * We very purposely don't print errors in mmc_regulator_set_vqmmc().
> > 
> >   There are cases where the MMC core will try several different
> >   voltages and we don't want to pollute the logs.
> > 
> > Signed-off-by: Douglas Anderson <dianders@chromium.org>
> > Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> 
> This looks good to me!

very cool :-)


> Once all are happy with the patches, can we take the mmc patches via
> my mmc tree or does it all have to go together?

The clock changes of course only touch internals of the phase-clocks, so 
should have no problem going through another tree.

For the devicetree part I'm unsure. If the boards enable the tuning-related 
settings without the new voltage handling, 2.7V gets set on all Rockchip 
boards which doesn't work on those at all.

So either the dts patches would need to go into your tree, I would need a 
stable branch or we could simply postpone dts changes for the next cycle.


Heiko

> > ---
> > 
> >  drivers/mmc/core/core.c  | 74
> >  ++++++++++++++++++++++++++++++++++++++++++++++++
> >  include/linux/mmc/host.h |  7 +++++
> >  2 files changed, 81 insertions(+)
> > 
> > diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> > index 0520064..363cc98 100644
> > --- a/drivers/mmc/core/core.c
> > +++ b/drivers/mmc/core/core.c
> > @@ -1437,6 +1437,80 @@ int mmc_regulator_set_ocr(struct mmc_host *mmc,
> > 
> >  }
> >  EXPORT_SYMBOL_GPL(mmc_regulator_set_ocr);
> > 
> > +static int mmc_regulator_set_voltage_if_supported(struct regulator
> > *regulator, +                                                 int min_uV,
> > int target_uV, +                                                 int
> > max_uV)
> > +{
> > +       /*
> > +        * Check if supported first to avoid errors since we may try
> > several +        * signal levels during power up and don't want to show
> > errors. +        */
> > +       if (!regulator_is_supported_voltage(regulator, min_uV, max_uV))
> > +               return -EINVAL;
> > +
> > +       return regulator_set_voltage_triplet(regulator, min_uV, target_uV,
> > +                                            max_uV);
> > +}
> > +
> > +/**
> > + * mmc_regulator_set_vqmmc - Set VQMMC as per the ios
> > + *
> > + * For 3.3V signaling, we try to match VQMMC to VMMC as closely as
> > possible. + * That will match the behavior of old boards where VQMMC and
> > VMMC were supplied + * by the same supply.  The Bus Operating conditions
> > for 3.3V signaling in the + * SD card spec also define VQMMC in terms of
> > VMMC.
> > + * If this is not possible we'll try the full 2.7-3.6V of the spec.
> > + *
> > + * For 1.2V and 1.8V signaling we'll try to get as close as possible to
> > the + * requested voltage.  This is definitely a good idea for UHS where
> > there's a + * separate regulator on the card that's trying to make 1.8V
> > and it's best if + * we match.
> > + *
> > + * This function is expected to be used by a controller's
> > + * start_signal_voltage_switch() function.
> > + */
> > +int mmc_regulator_set_vqmmc(struct mmc_host *mmc, struct mmc_ios *ios)
> > +{
> > +       int volt, min_uV, max_uV;
> > +
> > +       /* If no vqmmc supply then we can't change the voltage */
> > +       if (IS_ERR(mmc->supply.vqmmc))
> > +               return -EINVAL;
> > +
> > +       switch (ios->signal_voltage) {
> > +       case MMC_SIGNAL_VOLTAGE_120:
> > +               return
> > mmc_regulator_set_voltage_if_supported(mmc->supply.vqmmc, +              
> >                                 1100000, 1200000, 1300000); +       case
> > MMC_SIGNAL_VOLTAGE_180:
> > +               return
> > mmc_regulator_set_voltage_if_supported(mmc->supply.vqmmc, +              
> >                                 1700000, 1800000, 1950000); +       case
> > MMC_SIGNAL_VOLTAGE_330:
> > +               volt = regulator_get_voltage(mmc->supply.vmmc);
> > +               if (volt < 0)
> > +                       return volt;
> > +
> > +               min_uV = max(volt - 300000, 2700000);
> > +               max_uV = min(volt + 300000, 3600000);
> > +
> > +               /*
> > +                * Due to a limitation in the current implementation of
> > +                * regulator_set_voltage_triplet() which is taking the
> > lowest +                * voltage possible if below the target, search
> > for a suitable +                * voltage in two steps and try to stay
> > close to vmmc +                * with a 0.3V tolerance at first.
> > +                */
> > +               if
> > (!mmc_regulator_set_voltage_if_supported(mmc->supply.vqmmc, +            
> >                                   min_uV, volt, max_uV)) +               
> >        return 0;
> > +
> > +               return
> > mmc_regulator_set_voltage_if_supported(mmc->supply.vqmmc, +              
> >                                 2700000, volt, 3600000); +       default:
> > +               return -EINVAL;
> > +       }
> > +}
> > +EXPORT_SYMBOL_GPL(mmc_regulator_set_vqmmc);
> > +
> > 
> >  #endif /* CONFIG_REGULATOR */
> >  
> >  int mmc_regulator_get_supply(struct mmc_host *mmc)
> > 
> > diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> > index 83b81fd..a2a78eb 100644
> > --- a/include/linux/mmc/host.h
> > +++ b/include/linux/mmc/host.h
> > @@ -423,6 +423,7 @@ int mmc_regulator_get_ocrmask(struct regulator
> > *supply);> 
> >  int mmc_regulator_set_ocr(struct mmc_host *mmc,
> >  
> >                         struct regulator *supply,
> >                         unsigned short vdd_bit);
> > 
> > +int mmc_regulator_set_vqmmc(struct mmc_host *mmc, struct mmc_ios *ios);
> > 
> >  #else
> >  static inline int mmc_regulator_get_ocrmask(struct regulator *supply)
> >  {
> > 
> > @@ -435,6 +436,12 @@ static inline int mmc_regulator_set_ocr(struct
> > mmc_host *mmc,> 
> >  {
> >  
> >         return 0;
> >  
> >  }
> > 
> > +
> > +static inline int mmc_regulator_set_vqmmc(struct mmc_host *mmc,
> > +                                         struct mmc_ios *ios)
> > +{
> > +       return -EINVAL;
> > +}
> > 
> >  #endif
> >  
> >  int mmc_regulator_get_supply(struct mmc_host *mmc);
> > 
> > --
> > 2.5.1

Re: [PATCH v2 3/8] mmc: core: Add mmc_regulator_set_vqmmc()

[Ulf Hansson]

On 30 September 2015 at 16:55, Heiko St=C3=BCbner <heiko@sntech.de> wrote:
> Am Mittwoch, 30. September 2015, 16:42:05 schrieb Ulf Hansson:
>> On 30 September 2015 at 16:07, Heiko Stuebner <heiko@sntech.de> wrote:
>> > From: Douglas Anderson <dianders@chromium.org>
>> >
>> > This adds logic to the MMC core to set VQMMC.  This is expected to be
>> > called by MMC drivers like dw_mmc as part of (or instead of) their
>> > start_signal_voltage_switch() callback.
>> >
>> > A few notes:
>> >
>> > * When setting the signal voltage to 3.3V we do our best to make VQMMC
>> >
>> >   and VMMC match.  It's been reported that this makes some old cards
>> >   happy since they were tested back in the day before UHS when VQMMC
>> >   and VMMC were provided by the same regulator.  A nice side effect of
>> >   this is that we don't end up on the hairy edge of VQMMC (2.7V),
>> >   which some EEs claim is a little too close to the minimum for
>> >   comfort.
>> >   This is done in two steps. At first we try to find a VQMMC within
>> >   a 0.3V tolerance of VMMC and if this is not supported by the
>> >   supplying regulator we try to find a suitable voltage within the
>> >   whole 2.7V-3.6V area of the spec.
>> >
>> > * The two step approach is currently necessary, as the used
>> >
>> >   regulator_set_voltage_triplet(min, target, max) uses a simple
>> >
>> >   implementation that just tries two basic steps:
>> >         regulator_set_voltage(target, max);
>> >         regulator_set_voltage(min, target);
>> >
>> >   So with only one step with 2.7-3.6V borders, if a suitable voltage
>> >   is a bit below VMMC, we would directly get the lowest 2.7V
>> >   which some boards (like Rockchips) don't like at all.
>> >
>> > * When setting the signal voltage to 1.8V or 1.2V we aim for that
>> >
>> >   specific voltage instead of picking the lowest one in the range.
>> >
>> > * We very purposely don't print errors in mmc_regulator_set_vqmmc().
>> >
>> >   There are cases where the MMC core will try several different
>> >   voltages and we don't want to pollute the logs.
>> >
>> > Signed-off-by: Douglas Anderson <dianders@chromium.org>
>> > Signed-off-by: Heiko Stuebner <heiko@sntech.de>
>>
>> This looks good to me!
>
> very cool :-)
>
>
>> Once all are happy with the patches, can we take the mmc patches via
>> my mmc tree or does it all have to go together?
>
> The clock changes of course only touch internals of the phase-clocks, so
> should have no problem going through another tree.

What happens if I take mmc and dt changes, wouldn't I need the clock
patches as well?

>
> For the devicetree part I'm unsure. If the boards enable the tuning-relat=
ed
> settings without the new voltage handling, 2.7V gets set on all Rockchip
> boards which doesn't work on those at all.
>
> So either the dts patches would need to go into your tree, I would need a
> stable branch or we could simply postpone dts changes for the next cycle.
>

Kind regards
Uffe

Re: [PATCH v2 3/8] mmc: core: Add mmc_regulator_set_vqmmc()

[Ulf Hansson]

On 30 September 2015 at 16:55, Heiko Stübner <heiko@sntech.de> wrote:
> Am Mittwoch, 30. September 2015, 16:42:05 schrieb Ulf Hansson:
>> On 30 September 2015 at 16:07, Heiko Stuebner <heiko@sntech.de> wrote:
>> > From: Douglas Anderson <dianders@chromium.org>
>> >
>> > This adds logic to the MMC core to set VQMMC.  This is expected to be
>> > called by MMC drivers like dw_mmc as part of (or instead of) their
>> > start_signal_voltage_switch() callback.
>> >
>> > A few notes:
>> >
>> > * When setting the signal voltage to 3.3V we do our best to make VQMMC
>> >
>> >   and VMMC match.  It's been reported that this makes some old cards
>> >   happy since they were tested back in the day before UHS when VQMMC
>> >   and VMMC were provided by the same regulator.  A nice side effect of
>> >   this is that we don't end up on the hairy edge of VQMMC (2.7V),
>> >   which some EEs claim is a little too close to the minimum for
>> >   comfort.
>> >   This is done in two steps. At first we try to find a VQMMC within
>> >   a 0.3V tolerance of VMMC and if this is not supported by the
>> >   supplying regulator we try to find a suitable voltage within the
>> >   whole 2.7V-3.6V area of the spec.
>> >
>> > * The two step approach is currently necessary, as the used
>> >
>> >   regulator_set_voltage_triplet(min, target, max) uses a simple
>> >
>> >   implementation that just tries two basic steps:
>> >         regulator_set_voltage(target, max);
>> >         regulator_set_voltage(min, target);
>> >
>> >   So with only one step with 2.7-3.6V borders, if a suitable voltage
>> >   is a bit below VMMC, we would directly get the lowest 2.7V
>> >   which some boards (like Rockchips) don't like at all.
>> >
>> > * When setting the signal voltage to 1.8V or 1.2V we aim for that
>> >
>> >   specific voltage instead of picking the lowest one in the range.
>> >
>> > * We very purposely don't print errors in mmc_regulator_set_vqmmc().
>> >
>> >   There are cases where the MMC core will try several different
>> >   voltages and we don't want to pollute the logs.
>> >
>> > Signed-off-by: Douglas Anderson <dianders@chromium.org>
>> > Signed-off-by: Heiko Stuebner <heiko@sntech.de>
>>
>> This looks good to me!
>
> very cool :-)
>
>
>> Once all are happy with the patches, can we take the mmc patches via
>> my mmc tree or does it all have to go together?
>
> The clock changes of course only touch internals of the phase-clocks, so
> should have no problem going through another tree.

What happens if I take mmc and dt changes, wouldn't I need the clock
patches as well?

>
> For the devicetree part I'm unsure. If the boards enable the tuning-related
> settings without the new voltage handling, 2.7V gets set on all Rockchip
> boards which doesn't work on those at all.
>
> So either the dts patches would need to go into your tree, I would need a
> stable branch or we could simply postpone dts changes for the next cycle.
>

Kind regards
Uffe

[PATCH v2 3/8] mmc: core: Add mmc_regulator_set_vqmmc()

[Ulf Hansson]

On 30 September 2015 at 16:55, Heiko St?bner <heiko@sntech.de> wrote:
> Am Mittwoch, 30. September 2015, 16:42:05 schrieb Ulf Hansson:
>> On 30 September 2015 at 16:07, Heiko Stuebner <heiko@sntech.de> wrote:
>> > From: Douglas Anderson <dianders@chromium.org>
>> >
>> > This adds logic to the MMC core to set VQMMC.  This is expected to be
>> > called by MMC drivers like dw_mmc as part of (or instead of) their
>> > start_signal_voltage_switch() callback.
>> >
>> > A few notes:
>> >
>> > * When setting the signal voltage to 3.3V we do our best to make VQMMC
>> >
>> >   and VMMC match.  It's been reported that this makes some old cards
>> >   happy since they were tested back in the day before UHS when VQMMC
>> >   and VMMC were provided by the same regulator.  A nice side effect of
>> >   this is that we don't end up on the hairy edge of VQMMC (2.7V),
>> >   which some EEs claim is a little too close to the minimum for
>> >   comfort.
>> >   This is done in two steps. At first we try to find a VQMMC within
>> >   a 0.3V tolerance of VMMC and if this is not supported by the
>> >   supplying regulator we try to find a suitable voltage within the
>> >   whole 2.7V-3.6V area of the spec.
>> >
>> > * The two step approach is currently necessary, as the used
>> >
>> >   regulator_set_voltage_triplet(min, target, max) uses a simple
>> >
>> >   implementation that just tries two basic steps:
>> >         regulator_set_voltage(target, max);
>> >         regulator_set_voltage(min, target);
>> >
>> >   So with only one step with 2.7-3.6V borders, if a suitable voltage
>> >   is a bit below VMMC, we would directly get the lowest 2.7V
>> >   which some boards (like Rockchips) don't like at all.
>> >
>> > * When setting the signal voltage to 1.8V or 1.2V we aim for that
>> >
>> >   specific voltage instead of picking the lowest one in the range.
>> >
>> > * We very purposely don't print errors in mmc_regulator_set_vqmmc().
>> >
>> >   There are cases where the MMC core will try several different
>> >   voltages and we don't want to pollute the logs.
>> >
>> > Signed-off-by: Douglas Anderson <dianders@chromium.org>
>> > Signed-off-by: Heiko Stuebner <heiko@sntech.de>
>>
>> This looks good to me!
>
> very cool :-)
>
>
>> Once all are happy with the patches, can we take the mmc patches via
>> my mmc tree or does it all have to go together?
>
> The clock changes of course only touch internals of the phase-clocks, so
> should have no problem going through another tree.

What happens if I take mmc and dt changes, wouldn't I need the clock
patches as well?

>
> For the devicetree part I'm unsure. If the boards enable the tuning-related
> settings without the new voltage handling, 2.7V gets set on all Rockchip
> boards which doesn't work on those at all.
>
> So either the dts patches would need to go into your tree, I would need a
> stable branch or we could simply postpone dts changes for the next cycle.
>

Kind regards
Uffe

Re: [PATCH v2 3/8] mmc: core: Add mmc_regulator_set_vqmmc()

[Heiko Stübner]

Am Donnerstag, 1. Oktober 2015, 11:54:24 schrieb Ulf Hansson:
> On 30 September 2015 at 16:55, Heiko St=FCbner <heiko@sntech.de> wrot=
e:
> > Am Mittwoch, 30. September 2015, 16:42:05 schrieb Ulf Hansson:
> >> On 30 September 2015 at 16:07, Heiko Stuebner <heiko@sntech.de> wr=
ote:
> >> > From: Douglas Anderson <dianders@chromium.org>
> >> >=20
> >> > This adds logic to the MMC core to set VQMMC.  This is expected =
to be
> >> > called by MMC drivers like dw_mmc as part of (or instead of) the=
ir
> >> > start_signal_voltage_switch() callback.
> >> >=20
> >> > A few notes:
> >> >=20
> >> > * When setting the signal voltage to 3.3V we do our best to make=
 VQMMC
> >> >=20
> >> >   and VMMC match.  It's been reported that this makes some old c=
ards
> >> >   happy since they were tested back in the day before UHS when V=
QMMC
> >> >   and VMMC were provided by the same regulator.  A nice side eff=
ect of
> >> >   this is that we don't end up on the hairy edge of VQMMC (2.7V)=
,
> >> >   which some EEs claim is a little too close to the minimum for
> >> >   comfort.
> >> >   This is done in two steps. At first we try to find a VQMMC wit=
hin
> >> >   a 0.3V tolerance of VMMC and if this is not supported by the
> >> >   supplying regulator we try to find a suitable voltage within t=
he
> >> >   whole 2.7V-3.6V area of the spec.
> >> >=20
> >> > * The two step approach is currently necessary, as the used
> >> >=20
> >> >   regulator_set_voltage_triplet(min, target, max) uses a simple
> >> >  =20
> >> >   implementation that just tries two basic steps:
> >> >         regulator_set_voltage(target, max);
> >> >         regulator_set_voltage(min, target);
> >> >  =20
> >> >   So with only one step with 2.7-3.6V borders, if a suitable vol=
tage
> >> >   is a bit below VMMC, we would directly get the lowest 2.7V
> >> >   which some boards (like Rockchips) don't like at all.
> >> >=20
> >> > * When setting the signal voltage to 1.8V or 1.2V we aim for tha=
t
> >> >=20
> >> >   specific voltage instead of picking the lowest one in the rang=
e.
> >> >=20
> >> > * We very purposely don't print errors in mmc_regulator_set_vqmm=
c().
> >> >=20
> >> >   There are cases where the MMC core will try several different
> >> >   voltages and we don't want to pollute the logs.
> >> >=20
> >> > Signed-off-by: Douglas Anderson <dianders@chromium.org>
> >> > Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> >>=20
> >> This looks good to me!
> >=20
> > very cool :-)
> >=20
> >> Once all are happy with the patches, can we take the mmc patches v=
ia
> >> my mmc tree or does it all have to go together?
> >=20
> > The clock changes of course only touch internals of the phase-clock=
s, so
> > should have no problem going through another tree.
>=20
> What happens if I take mmc and dt changes, wouldn't I need the clock
> patches as well?

The API stays of course the same, only the degree to settings translati=
on gets=20
optimized, so I guess in the worst case you would get no good phase and=
 thus=20
fall back to non-highspeed modes - but the system would stay running.

But of course, if the clock maintainers could Ack the two clock patches=
 and=20
everything would stay together that would work even better :-)


Heiko

>=20
> > For the devicetree part I'm unsure. If the boards enable the
> > tuning-related
> > settings without the new voltage handling, 2.7V gets set on all Roc=
kchip
> > boards which doesn't work on those at all.
> >=20
> > So either the dts patches would need to go into your tree, I would =
need a
> > stable branch or we could simply postpone dts changes for the next =
cycle.
>=20
> Kind regards
> Uffe

Re: [PATCH v2 3/8] mmc: core: Add mmc_regulator_set_vqmmc()

[Heiko Stübner]

Am Donnerstag, 1. Oktober 2015, 11:54:24 schrieb Ulf Hansson:
> On 30 September 2015 at 16:55, Heiko Stübner <heiko@sntech.de> wrote:
> > Am Mittwoch, 30. September 2015, 16:42:05 schrieb Ulf Hansson:
> >> On 30 September 2015 at 16:07, Heiko Stuebner <heiko@sntech.de> wrote:
> >> > From: Douglas Anderson <dianders@chromium.org>
> >> > 
> >> > This adds logic to the MMC core to set VQMMC.  This is expected to be
> >> > called by MMC drivers like dw_mmc as part of (or instead of) their
> >> > start_signal_voltage_switch() callback.
> >> > 
> >> > A few notes:
> >> > 
> >> > * When setting the signal voltage to 3.3V we do our best to make VQMMC
> >> > 
> >> >   and VMMC match.  It's been reported that this makes some old cards
> >> >   happy since they were tested back in the day before UHS when VQMMC
> >> >   and VMMC were provided by the same regulator.  A nice side effect of
> >> >   this is that we don't end up on the hairy edge of VQMMC (2.7V),
> >> >   which some EEs claim is a little too close to the minimum for
> >> >   comfort.
> >> >   This is done in two steps. At first we try to find a VQMMC within
> >> >   a 0.3V tolerance of VMMC and if this is not supported by the
> >> >   supplying regulator we try to find a suitable voltage within the
> >> >   whole 2.7V-3.6V area of the spec.
> >> > 
> >> > * The two step approach is currently necessary, as the used
> >> > 
> >> >   regulator_set_voltage_triplet(min, target, max) uses a simple
> >> >   
> >> >   implementation that just tries two basic steps:
> >> >         regulator_set_voltage(target, max);
> >> >         regulator_set_voltage(min, target);
> >> >   
> >> >   So with only one step with 2.7-3.6V borders, if a suitable voltage
> >> >   is a bit below VMMC, we would directly get the lowest 2.7V
> >> >   which some boards (like Rockchips) don't like at all.
> >> > 
> >> > * When setting the signal voltage to 1.8V or 1.2V we aim for that
> >> > 
> >> >   specific voltage instead of picking the lowest one in the range.
> >> > 
> >> > * We very purposely don't print errors in mmc_regulator_set_vqmmc().
> >> > 
> >> >   There are cases where the MMC core will try several different
> >> >   voltages and we don't want to pollute the logs.
> >> > 
> >> > Signed-off-by: Douglas Anderson <dianders@chromium.org>
> >> > Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> >> 
> >> This looks good to me!
> > 
> > very cool :-)
> > 
> >> Once all are happy with the patches, can we take the mmc patches via
> >> my mmc tree or does it all have to go together?
> > 
> > The clock changes of course only touch internals of the phase-clocks, so
> > should have no problem going through another tree.
> 
> What happens if I take mmc and dt changes, wouldn't I need the clock
> patches as well?

The API stays of course the same, only the degree to settings translation gets 
optimized, so I guess in the worst case you would get no good phase and thus 
fall back to non-highspeed modes - but the system would stay running.

But of course, if the clock maintainers could Ack the two clock patches and 
everything would stay together that would work even better :-)


Heiko

> 
> > For the devicetree part I'm unsure. If the boards enable the
> > tuning-related
> > settings without the new voltage handling, 2.7V gets set on all Rockchip
> > boards which doesn't work on those at all.
> > 
> > So either the dts patches would need to go into your tree, I would need a
> > stable branch or we could simply postpone dts changes for the next cycle.
> 
> Kind regards
> Uffe

[PATCH v2 3/8] mmc: core: Add mmc_regulator_set_vqmmc()

[Heiko Stübner]

Am Donnerstag, 1. Oktober 2015, 11:54:24 schrieb Ulf Hansson:
> On 30 September 2015 at 16:55, Heiko St?bner <heiko@sntech.de> wrote:
> > Am Mittwoch, 30. September 2015, 16:42:05 schrieb Ulf Hansson:
> >> On 30 September 2015 at 16:07, Heiko Stuebner <heiko@sntech.de> wrote:
> >> > From: Douglas Anderson <dianders@chromium.org>
> >> > 
> >> > This adds logic to the MMC core to set VQMMC.  This is expected to be
> >> > called by MMC drivers like dw_mmc as part of (or instead of) their
> >> > start_signal_voltage_switch() callback.
> >> > 
> >> > A few notes:
> >> > 
> >> > * When setting the signal voltage to 3.3V we do our best to make VQMMC
> >> > 
> >> >   and VMMC match.  It's been reported that this makes some old cards
> >> >   happy since they were tested back in the day before UHS when VQMMC
> >> >   and VMMC were provided by the same regulator.  A nice side effect of
> >> >   this is that we don't end up on the hairy edge of VQMMC (2.7V),
> >> >   which some EEs claim is a little too close to the minimum for
> >> >   comfort.
> >> >   This is done in two steps. At first we try to find a VQMMC within
> >> >   a 0.3V tolerance of VMMC and if this is not supported by the
> >> >   supplying regulator we try to find a suitable voltage within the
> >> >   whole 2.7V-3.6V area of the spec.
> >> > 
> >> > * The two step approach is currently necessary, as the used
> >> > 
> >> >   regulator_set_voltage_triplet(min, target, max) uses a simple
> >> >   
> >> >   implementation that just tries two basic steps:
> >> >         regulator_set_voltage(target, max);
> >> >         regulator_set_voltage(min, target);
> >> >   
> >> >   So with only one step with 2.7-3.6V borders, if a suitable voltage
> >> >   is a bit below VMMC, we would directly get the lowest 2.7V
> >> >   which some boards (like Rockchips) don't like at all.
> >> > 
> >> > * When setting the signal voltage to 1.8V or 1.2V we aim for that
> >> > 
> >> >   specific voltage instead of picking the lowest one in the range.
> >> > 
> >> > * We very purposely don't print errors in mmc_regulator_set_vqmmc().
> >> > 
> >> >   There are cases where the MMC core will try several different
> >> >   voltages and we don't want to pollute the logs.
> >> > 
> >> > Signed-off-by: Douglas Anderson <dianders@chromium.org>
> >> > Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> >> 
> >> This looks good to me!
> > 
> > very cool :-)
> > 
> >> Once all are happy with the patches, can we take the mmc patches via
> >> my mmc tree or does it all have to go together?
> > 
> > The clock changes of course only touch internals of the phase-clocks, so
> > should have no problem going through another tree.
> 
> What happens if I take mmc and dt changes, wouldn't I need the clock
> patches as well?

The API stays of course the same, only the degree to settings translation gets 
optimized, so I guess in the worst case you would get no good phase and thus 
fall back to non-highspeed modes - but the system would stay running.

But of course, if the clock maintainers could Ack the two clock patches and 
everything would stay together that would work even better :-)


Heiko

> 
> > For the devicetree part I'm unsure. If the boards enable the
> > tuning-related
> > settings without the new voltage handling, 2.7V gets set on all Rockchip
> > boards which doesn't work on those at all.
> > 
> > So either the dts patches would need to go into your tree, I would need a
> > stable branch or we could simply postpone dts changes for the next cycle.
> 
> Kind regards
> Uffe

Re: [PATCH v2 3/8] mmc: core: Add mmc_regulator_set_vqmmc()

[Stephen Boyd]

On 10/01, Heiko Stübner wrote:
> Am Donnerstag, 1. Oktober 2015, 11:54:24 schrieb Ulf Hansson:
> > On 30 September 2015 at 16:55, Heiko Stübner <heiko@sntech.de> wrote:
> > > Am Mittwoch, 30. September 2015, 16:42:05 schrieb Ulf Hansson:
> > >> On 30 September 2015 at 16:07, Heiko Stuebner <heiko@sntech.de> wrote:
> > > The clock changes of course only touch internals of the phase-clocks, so
> > > should have no problem going through another tree.
> > 
> > What happens if I take mmc and dt changes, wouldn't I need the clock
> > patches as well?
> 
> The API stays of course the same, only the degree to settings translation gets 
> optimized, so I guess in the worst case you would get no good phase and thus 
> fall back to non-highspeed modes - but the system would stay running.
> 
> But of course, if the clock maintainers could Ack the two clock patches and 
> everything would stay together that would work even better :-)
> 

If Ulf doesn't want to take them we can apply them to clk tree.
Otherwise, you can have my acked-by on the clk patches.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v2 3/8] mmc: core: Add mmc_regulator_set_vqmmc()

[Stephen Boyd]

On 10/01, Heiko St?bner wrote:
> Am Donnerstag, 1. Oktober 2015, 11:54:24 schrieb Ulf Hansson:
> > On 30 September 2015 at 16:55, Heiko St?bner <heiko@sntech.de> wrote:
> > > Am Mittwoch, 30. September 2015, 16:42:05 schrieb Ulf Hansson:
> > >> On 30 September 2015 at 16:07, Heiko Stuebner <heiko@sntech.de> wrote:
> > > The clock changes of course only touch internals of the phase-clocks, so
> > > should have no problem going through another tree.
> > 
> > What happens if I take mmc and dt changes, wouldn't I need the clock
> > patches as well?
> 
> The API stays of course the same, only the degree to settings translation gets 
> optimized, so I guess in the worst case you would get no good phase and thus 
> fall back to non-highspeed modes - but the system would stay running.
> 
> But of course, if the clock maintainers could Ack the two clock patches and 
> everything would stay together that would work even better :-)
> 

If Ulf doesn't want to take them we can apply them to clk tree.
Otherwise, you can have my acked-by on the clk patches.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v2 3/8] mmc: core: Add mmc_regulator_set_vqmmc()

[Ulf Hansson]

On 1 October 2015 at 19:35, Stephen Boyd <sboyd@codeaurora.org> wrote:
> On 10/01, Heiko St=C3=BCbner wrote:
>> Am Donnerstag, 1. Oktober 2015, 11:54:24 schrieb Ulf Hansson:
>> > On 30 September 2015 at 16:55, Heiko St=C3=BCbner <heiko@sntech.de> wr=
ote:
>> > > Am Mittwoch, 30. September 2015, 16:42:05 schrieb Ulf Hansson:
>> > >> On 30 September 2015 at 16:07, Heiko Stuebner <heiko@sntech.de> wro=
te:
>> > > The clock changes of course only touch internals of the phase-clocks=
, so
>> > > should have no problem going through another tree.
>> >
>> > What happens if I take mmc and dt changes, wouldn't I need the clock
>> > patches as well?
>>
>> The API stays of course the same, only the degree to settings translatio=
n gets
>> optimized, so I guess in the worst case you would get no good phase and =
thus
>> fall back to non-highspeed modes - but the system would stay running.
>>
>> But of course, if the clock maintainers could Ack the two clock patches =
and
>> everything would stay together that would work even better :-)
>>
>
> If Ulf doesn't want to take them we can apply them to clk tree.
> Otherwise, you can have my acked-by on the clk patches.

I don't mind picking up the clock patches. So I consider this as an
ack for both patch 1 and patch2, thanks.

Now, let's give Jaehoon some time to review the dw_mmc parts.

Kind regards
Uffe

Re: [PATCH v2 3/8] mmc: core: Add mmc_regulator_set_vqmmc()

[Ulf Hansson]

On 1 October 2015 at 19:35, Stephen Boyd <sboyd@codeaurora.org> wrote:
> On 10/01, Heiko Stübner wrote:
>> Am Donnerstag, 1. Oktober 2015, 11:54:24 schrieb Ulf Hansson:
>> > On 30 September 2015 at 16:55, Heiko Stübner <heiko@sntech.de> wrote:
>> > > Am Mittwoch, 30. September 2015, 16:42:05 schrieb Ulf Hansson:
>> > >> On 30 September 2015 at 16:07, Heiko Stuebner <heiko@sntech.de> wrote:
>> > > The clock changes of course only touch internals of the phase-clocks, so
>> > > should have no problem going through another tree.
>> >
>> > What happens if I take mmc and dt changes, wouldn't I need the clock
>> > patches as well?
>>
>> The API stays of course the same, only the degree to settings translation gets
>> optimized, so I guess in the worst case you would get no good phase and thus
>> fall back to non-highspeed modes - but the system would stay running.
>>
>> But of course, if the clock maintainers could Ack the two clock patches and
>> everything would stay together that would work even better :-)
>>
>
> If Ulf doesn't want to take them we can apply them to clk tree.
> Otherwise, you can have my acked-by on the clk patches.

I don't mind picking up the clock patches. So I consider this as an
ack for both patch 1 and patch2, thanks.

Now, let's give Jaehoon some time to review the dw_mmc parts.

Kind regards
Uffe

[PATCH v2 3/8] mmc: core: Add mmc_regulator_set_vqmmc()

[Ulf Hansson]

On 1 October 2015 at 19:35, Stephen Boyd <sboyd@codeaurora.org> wrote:
> On 10/01, Heiko St?bner wrote:
>> Am Donnerstag, 1. Oktober 2015, 11:54:24 schrieb Ulf Hansson:
>> > On 30 September 2015 at 16:55, Heiko St?bner <heiko@sntech.de> wrote:
>> > > Am Mittwoch, 30. September 2015, 16:42:05 schrieb Ulf Hansson:
>> > >> On 30 September 2015 at 16:07, Heiko Stuebner <heiko@sntech.de> wrote:
>> > > The clock changes of course only touch internals of the phase-clocks, so
>> > > should have no problem going through another tree.
>> >
>> > What happens if I take mmc and dt changes, wouldn't I need the clock
>> > patches as well?
>>
>> The API stays of course the same, only the degree to settings translation gets
>> optimized, so I guess in the worst case you would get no good phase and thus
>> fall back to non-highspeed modes - but the system would stay running.
>>
>> But of course, if the clock maintainers could Ack the two clock patches and
>> everything would stay together that would work even better :-)
>>
>
> If Ulf doesn't want to take them we can apply them to clk tree.
> Otherwise, you can have my acked-by on the clk patches.

I don't mind picking up the clock patches. So I consider this as an
ack for both patch 1 and patch2, thanks.

Now, let's give Jaehoon some time to review the dw_mmc parts.

Kind regards
Uffe

Re: [PATCH v2 3/8] mmc: core: Add mmc_regulator_set_vqmmc()

[Jaehoon Chung]

On 10/02/2015 06:05 AM, Ulf Hansson wrote:
> On 1 October 2015 at 19:35, Stephen Boyd <sboyd@codeaurora.org> wrote:
>> On 10/01, Heiko St=C3=BCbner wrote:
>>> Am Donnerstag, 1. Oktober 2015, 11:54:24 schrieb Ulf Hansson:
>>>> On 30 September 2015 at 16:55, Heiko St=C3=BCbner <heiko@sntech.de> wrote:
>>>>> Am Mittwoch, 30. September 2015, 16:42:05 schrieb Ulf Hansson:
>>>>>> On 30 September 2015 at 16:07, Heiko Stuebner <heiko@sntech.de> wrote:
>>>>> The clock changes of course only touch internals of the phase-clocks, so
>>>>> should have no problem going through another tree.
>>>>
>>>> What happens if I take mmc and dt changes, wouldn't I need the clock
>>>> patches as well=3F
>>>
>>> The API stays of course the same, only the degree to settings translation gets
>>> optimized, so I guess in the worst case you would get no good phase and thus
>>> fall back to non-highspeed modes - but the system would stay running.
>>>
>>> But of course, if the clock maintainers could Ack the two clock patches and
>>> everything would stay together that would work even better :-)
>>>
>>
>> If Ulf doesn't want to take them we can apply them to clk tree.
>> Otherwise, you can have my acked-by on the clk patches.
> 
> I don't mind picking up the clock patches. So I consider this as an
> ack for both patch 1 and patch2, thanks.
> 
> Now, let's give Jaehoon some time to review the dw_mmc parts.

I will check other patches on today, if it's ok, i will apply at my repository.
Thanks for giving time! :)

Best Regards,
Jaehoon Chung

> 
> Kind regards
> Uffe
> 

Re: [PATCH v2 3/8] mmc: core: Add mmc_regulator_set_vqmmc()

[Jaehoon Chung]

On 10/02/2015 06:05 AM, Ulf Hansson wrote:
> On 1 October 2015 at 19:35, Stephen Boyd <sboyd@codeaurora.org> wrote:
>> On 10/01, Heiko Stübner wrote:
>>> Am Donnerstag, 1. Oktober 2015, 11:54:24 schrieb Ulf Hansson:
>>>> On 30 September 2015 at 16:55, Heiko Stübner <heiko@sntech.de> wrote:
>>>>> Am Mittwoch, 30. September 2015, 16:42:05 schrieb Ulf Hansson:
>>>>>> On 30 September 2015 at 16:07, Heiko Stuebner <heiko@sntech.de> wrote:
>>>>> The clock changes of course only touch internals of the phase-clocks, so
>>>>> should have no problem going through another tree.
>>>>
>>>> What happens if I take mmc and dt changes, wouldn't I need the clock
>>>> patches as well?
>>>
>>> The API stays of course the same, only the degree to settings translation gets
>>> optimized, so I guess in the worst case you would get no good phase and thus
>>> fall back to non-highspeed modes - but the system would stay running.
>>>
>>> But of course, if the clock maintainers could Ack the two clock patches and
>>> everything would stay together that would work even better :-)
>>>
>>
>> If Ulf doesn't want to take them we can apply them to clk tree.
>> Otherwise, you can have my acked-by on the clk patches.
> 
> I don't mind picking up the clock patches. So I consider this as an
> ack for both patch 1 and patch2, thanks.
> 
> Now, let's give Jaehoon some time to review the dw_mmc parts.

I will check other patches on today, if it's ok, i will apply at my repository.
Thanks for giving time! :)

Best Regards,
Jaehoon Chung

> 
> Kind regards
> Uffe
> 

[PATCH v2 3/8] mmc: core: Add mmc_regulator_set_vqmmc()

[Jaehoon Chung]

On 10/02/2015 06:05 AM, Ulf Hansson wrote:
> On 1 October 2015 at 19:35, Stephen Boyd <sboyd@codeaurora.org> wrote:
>> On 10/01, Heiko St?bner wrote:
>>> Am Donnerstag, 1. Oktober 2015, 11:54:24 schrieb Ulf Hansson:
>>>> On 30 September 2015 at 16:55, Heiko St?bner <heiko@sntech.de> wrote:
>>>>> Am Mittwoch, 30. September 2015, 16:42:05 schrieb Ulf Hansson:
>>>>>> On 30 September 2015 at 16:07, Heiko Stuebner <heiko@sntech.de> wrote:
>>>>> The clock changes of course only touch internals of the phase-clocks, so
>>>>> should have no problem going through another tree.
>>>>
>>>> What happens if I take mmc and dt changes, wouldn't I need the clock
>>>> patches as well?
>>>
>>> The API stays of course the same, only the degree to settings translation gets
>>> optimized, so I guess in the worst case you would get no good phase and thus
>>> fall back to non-highspeed modes - but the system would stay running.
>>>
>>> But of course, if the clock maintainers could Ack the two clock patches and
>>> everything would stay together that would work even better :-)
>>>
>>
>> If Ulf doesn't want to take them we can apply them to clk tree.
>> Otherwise, you can have my acked-by on the clk patches.
> 
> I don't mind picking up the clock patches. So I consider this as an
> ack for both patch 1 and patch2, thanks.
> 
> Now, let's give Jaehoon some time to review the dw_mmc parts.

I will check other patches on today, if it's ok, i will apply at my repository.
Thanks for giving time! :)

Best Regards,
Jaehoon Chung

> 
> Kind regards
> Uffe
> 

Re: [PATCH v2 3/8] mmc: core: Add mmc_regulator_set_vqmmc()

[Heiko Stübner]

Am Freitag, 2. Oktober 2015, 11:52:00 schrieb Jaehoon Chung:
> On 10/02/2015 06:05 AM, Ulf Hansson wrote:
> > On 1 October 2015 at 19:35, Stephen Boyd <sboyd@codeaurora.org> wro=
te:
> >> On 10/01, Heiko St=FCbner wrote:
> >>> Am Donnerstag, 1. Oktober 2015, 11:54:24 schrieb Ulf Hansson:
> >>>> On 30 September 2015 at 16:55, Heiko St=FCbner <heiko@sntech.de>=
 wrote:
> >>>>> Am Mittwoch, 30. September 2015, 16:42:05 schrieb Ulf Hansson:
> >>>>>> On 30 September 2015 at 16:07, Heiko Stuebner <heiko@sntech.de=
>=20
wrote:
> >>>>> The clock changes of course only touch internals of the phase-c=
locks,
> >>>>> so
> >>>>> should have no problem going through another tree.
> >>>>=20
> >>>> What happens if I take mmc and dt changes, wouldn't I need the c=
lock
> >>>> patches as well?
> >>>=20
> >>> The API stays of course the same, only the degree to settings
> >>> translation gets optimized, so I guess in the worst case you woul=
d get
> >>> no good phase and thus fall back to non-highspeed modes - but the=

> >>> system would stay running.
> >>>=20
> >>> But of course, if the clock maintainers could Ack the two clock p=
atches
> >>> and
> >>> everything would stay together that would work even better :-)
> >>=20
> >> If Ulf doesn't want to take them we can apply them to clk tree.
> >> Otherwise, you can have my acked-by on the clk patches.
> >=20
> > I don't mind picking up the clock patches. So I consider this as an=

> > ack for both patch 1 and patch2, thanks.
> >=20
> > Now, let's give Jaehoon some time to review the dw_mmc parts.
>=20
> I will check other patches on today, if it's ok, i will apply at my
> repository. Thanks for giving time! :)

I think Ulf wanted to apply the whole series via the mmc tree directly,=
 but I=20
guess that is between you two to decide ;-)


Heiko

Re: [PATCH v2 3/8] mmc: core: Add mmc_regulator_set_vqmmc()

[Heiko Stübner]

Am Freitag, 2. Oktober 2015, 11:52:00 schrieb Jaehoon Chung:
> On 10/02/2015 06:05 AM, Ulf Hansson wrote:
> > On 1 October 2015 at 19:35, Stephen Boyd <sboyd@codeaurora.org> wrote:
> >> On 10/01, Heiko Stübner wrote:
> >>> Am Donnerstag, 1. Oktober 2015, 11:54:24 schrieb Ulf Hansson:
> >>>> On 30 September 2015 at 16:55, Heiko Stübner <heiko@sntech.de> wrote:
> >>>>> Am Mittwoch, 30. September 2015, 16:42:05 schrieb Ulf Hansson:
> >>>>>> On 30 September 2015 at 16:07, Heiko Stuebner <heiko@sntech.de> 
wrote:
> >>>>> The clock changes of course only touch internals of the phase-clocks,
> >>>>> so
> >>>>> should have no problem going through another tree.
> >>>> 
> >>>> What happens if I take mmc and dt changes, wouldn't I need the clock
> >>>> patches as well?
> >>> 
> >>> The API stays of course the same, only the degree to settings
> >>> translation gets optimized, so I guess in the worst case you would get
> >>> no good phase and thus fall back to non-highspeed modes - but the
> >>> system would stay running.
> >>> 
> >>> But of course, if the clock maintainers could Ack the two clock patches
> >>> and
> >>> everything would stay together that would work even better :-)
> >> 
> >> If Ulf doesn't want to take them we can apply them to clk tree.
> >> Otherwise, you can have my acked-by on the clk patches.
> > 
> > I don't mind picking up the clock patches. So I consider this as an
> > ack for both patch 1 and patch2, thanks.
> > 
> > Now, let's give Jaehoon some time to review the dw_mmc parts.
> 
> I will check other patches on today, if it's ok, i will apply at my
> repository. Thanks for giving time! :)

I think Ulf wanted to apply the whole series via the mmc tree directly, but I 
guess that is between you two to decide ;-)


Heiko

[PATCH v2 3/8] mmc: core: Add mmc_regulator_set_vqmmc()

[Heiko Stübner]

Am Freitag, 2. Oktober 2015, 11:52:00 schrieb Jaehoon Chung:
> On 10/02/2015 06:05 AM, Ulf Hansson wrote:
> > On 1 October 2015 at 19:35, Stephen Boyd <sboyd@codeaurora.org> wrote:
> >> On 10/01, Heiko St?bner wrote:
> >>> Am Donnerstag, 1. Oktober 2015, 11:54:24 schrieb Ulf Hansson:
> >>>> On 30 September 2015 at 16:55, Heiko St?bner <heiko@sntech.de> wrote:
> >>>>> Am Mittwoch, 30. September 2015, 16:42:05 schrieb Ulf Hansson:
> >>>>>> On 30 September 2015 at 16:07, Heiko Stuebner <heiko@sntech.de> 
wrote:
> >>>>> The clock changes of course only touch internals of the phase-clocks,
> >>>>> so
> >>>>> should have no problem going through another tree.
> >>>> 
> >>>> What happens if I take mmc and dt changes, wouldn't I need the clock
> >>>> patches as well?
> >>> 
> >>> The API stays of course the same, only the degree to settings
> >>> translation gets optimized, so I guess in the worst case you would get
> >>> no good phase and thus fall back to non-highspeed modes - but the
> >>> system would stay running.
> >>> 
> >>> But of course, if the clock maintainers could Ack the two clock patches
> >>> and
> >>> everything would stay together that would work even better :-)
> >> 
> >> If Ulf doesn't want to take them we can apply them to clk tree.
> >> Otherwise, you can have my acked-by on the clk patches.
> > 
> > I don't mind picking up the clock patches. So I consider this as an
> > ack for both patch 1 and patch2, thanks.
> > 
> > Now, let's give Jaehoon some time to review the dw_mmc parts.
> 
> I will check other patches on today, if it's ok, i will apply at my
> repository. Thanks for giving time! :)

I think Ulf wanted to apply the whole series via the mmc tree directly, but I 
guess that is between you two to decide ;-)


Heiko

Re: [PATCH v2 3/8] mmc: core: Add mmc_regulator_set_vqmmc()

[Jaehoon Chung]

On 10/02/2015 04:06 PM, Heiko Stübner wrote:
> Am Freitag, 2. Oktober 2015, 11:52:00 schrieb Jaehoon Chung:
>> On 10/02/2015 06:05 AM, Ulf Hansson wrote:
>>> On 1 October 2015 at 19:35, Stephen Boyd <sboyd@codeaurora.org> wrote:
>>>> On 10/01, Heiko Stübner wrote:
>>>>> Am Donnerstag, 1. Oktober 2015, 11:54:24 schrieb Ulf Hansson:
>>>>>> On 30 September 2015 at 16:55, Heiko Stübner <heiko@sntech.de> wrote:
>>>>>>> Am Mittwoch, 30. September 2015, 16:42:05 schrieb Ulf Hansson:
>>>>>>>> On 30 September 2015 at 16:07, Heiko Stuebner <heiko@sntech.de> 
> wrote:
>>>>>>> The clock changes of course only touch internals of the phase-clocks,
>>>>>>> so
>>>>>>> should have no problem going through another tree.
>>>>>>
>>>>>> What happens if I take mmc and dt changes, wouldn't I need the clock
>>>>>> patches as well?
>>>>>
>>>>> The API stays of course the same, only the degree to settings
>>>>> translation gets optimized, so I guess in the worst case you would get
>>>>> no good phase and thus fall back to non-highspeed modes - but the
>>>>> system would stay running.
>>>>>
>>>>> But of course, if the clock maintainers could Ack the two clock patches
>>>>> and
>>>>> everything would stay together that would work even better :-)
>>>>
>>>> If Ulf doesn't want to take them we can apply them to clk tree.
>>>> Otherwise, you can have my acked-by on the clk patches.
>>>
>>> I don't mind picking up the clock patches. So I consider this as an
>>> ack for both patch 1 and patch2, thanks.
>>>
>>> Now, let's give Jaehoon some time to review the dw_mmc parts.
>>
>> I will check other patches on today, if it's ok, i will apply at my
>> repository. Thanks for giving time! :)
> 
> I think Ulf wanted to apply the whole series via the mmc tree directly, but I 
> guess that is between you two to decide ;-)

If it will get Stephen's acked-by, i will pick up all after checked the dwmmc patches. how about?

Best Regards,
Jaehoon Chung

> 
> 
> Heiko
> 

[PATCH v2 3/8] mmc: core: Add mmc_regulator_set_vqmmc()

[Jaehoon Chung]

On 10/02/2015 04:06 PM, Heiko St?bner wrote:
> Am Freitag, 2. Oktober 2015, 11:52:00 schrieb Jaehoon Chung:
>> On 10/02/2015 06:05 AM, Ulf Hansson wrote:
>>> On 1 October 2015 at 19:35, Stephen Boyd <sboyd@codeaurora.org> wrote:
>>>> On 10/01, Heiko St?bner wrote:
>>>>> Am Donnerstag, 1. Oktober 2015, 11:54:24 schrieb Ulf Hansson:
>>>>>> On 30 September 2015 at 16:55, Heiko St?bner <heiko@sntech.de> wrote:
>>>>>>> Am Mittwoch, 30. September 2015, 16:42:05 schrieb Ulf Hansson:
>>>>>>>> On 30 September 2015 at 16:07, Heiko Stuebner <heiko@sntech.de> 
> wrote:
>>>>>>> The clock changes of course only touch internals of the phase-clocks,
>>>>>>> so
>>>>>>> should have no problem going through another tree.
>>>>>>
>>>>>> What happens if I take mmc and dt changes, wouldn't I need the clock
>>>>>> patches as well?
>>>>>
>>>>> The API stays of course the same, only the degree to settings
>>>>> translation gets optimized, so I guess in the worst case you would get
>>>>> no good phase and thus fall back to non-highspeed modes - but the
>>>>> system would stay running.
>>>>>
>>>>> But of course, if the clock maintainers could Ack the two clock patches
>>>>> and
>>>>> everything would stay together that would work even better :-)
>>>>
>>>> If Ulf doesn't want to take them we can apply them to clk tree.
>>>> Otherwise, you can have my acked-by on the clk patches.
>>>
>>> I don't mind picking up the clock patches. So I consider this as an
>>> ack for both patch 1 and patch2, thanks.
>>>
>>> Now, let's give Jaehoon some time to review the dw_mmc parts.
>>
>> I will check other patches on today, if it's ok, i will apply at my
>> repository. Thanks for giving time! :)
> 
> I think Ulf wanted to apply the whole series via the mmc tree directly, but I 
> guess that is between you two to decide ;-)

If it will get Stephen's acked-by, i will pick up all after checked the dwmmc patches. how about?

Best Regards,
Jaehoon Chung

> 
> 
> Heiko
> 

Re: [PATCH v2 3/8] mmc: core: Add mmc_regulator_set_vqmmc()

[Ulf Hansson]

On 2 October 2015 at 09:53, Jaehoon Chung <jh80.chung@samsung.com> wrote:
> On 10/02/2015 04:06 PM, Heiko St=C3=BCbner wrote:
>> Am Freitag, 2. Oktober 2015, 11:52:00 schrieb Jaehoon Chung:
>>> On 10/02/2015 06:05 AM, Ulf Hansson wrote:
>>>> On 1 October 2015 at 19:35, Stephen Boyd <sboyd@codeaurora.org> wrote:
>>>>> On 10/01, Heiko St=C3=BCbner wrote:
>>>>>> Am Donnerstag, 1. Oktober 2015, 11:54:24 schrieb Ulf Hansson:
>>>>>>> On 30 September 2015 at 16:55, Heiko St=C3=BCbner <heiko@sntech.de>=
 wrote:
>>>>>>>> Am Mittwoch, 30. September 2015, 16:42:05 schrieb Ulf Hansson:
>>>>>>>>> On 30 September 2015 at 16:07, Heiko Stuebner <heiko@sntech.de>
>> wrote:
>>>>>>>> The clock changes of course only touch internals of the phase-cloc=
ks,
>>>>>>>> so
>>>>>>>> should have no problem going through another tree.
>>>>>>>
>>>>>>> What happens if I take mmc and dt changes, wouldn't I need the cloc=
k
>>>>>>> patches as well?
>>>>>>
>>>>>> The API stays of course the same, only the degree to settings
>>>>>> translation gets optimized, so I guess in the worst case you would g=
et
>>>>>> no good phase and thus fall back to non-highspeed modes - but the
>>>>>> system would stay running.
>>>>>>
>>>>>> But of course, if the clock maintainers could Ack the two clock patc=
hes
>>>>>> and
>>>>>> everything would stay together that would work even better :-)
>>>>>
>>>>> If Ulf doesn't want to take them we can apply them to clk tree.
>>>>> Otherwise, you can have my acked-by on the clk patches.
>>>>
>>>> I don't mind picking up the clock patches. So I consider this as an
>>>> ack for both patch 1 and patch2, thanks.
>>>>
>>>> Now, let's give Jaehoon some time to review the dw_mmc parts.
>>>
>>> I will check other patches on today, if it's ok, i will apply at my
>>> repository. Thanks for giving time! :)
>>
>> I think Ulf wanted to apply the whole series via the mmc tree directly, =
but I
>> guess that is between you two to decide ;-)
>
> If it will get Stephen's acked-by, i will pick up all after checked the d=
wmmc patches. how about?

Stephen already acked the clock patches from this thread.

Perhaps this time, it's easier if you ack the dw_mmc patches and I can
pick up the hole series? Does that work for you?

Kind regards
Uffe

Re: [PATCH v2 3/8] mmc: core: Add mmc_regulator_set_vqmmc()

[Ulf Hansson]

On 2 October 2015 at 09:53, Jaehoon Chung <jh80.chung@samsung.com> wrote:
> On 10/02/2015 04:06 PM, Heiko Stübner wrote:
>> Am Freitag, 2. Oktober 2015, 11:52:00 schrieb Jaehoon Chung:
>>> On 10/02/2015 06:05 AM, Ulf Hansson wrote:
>>>> On 1 October 2015 at 19:35, Stephen Boyd <sboyd@codeaurora.org> wrote:
>>>>> On 10/01, Heiko Stübner wrote:
>>>>>> Am Donnerstag, 1. Oktober 2015, 11:54:24 schrieb Ulf Hansson:
>>>>>>> On 30 September 2015 at 16:55, Heiko Stübner <heiko@sntech.de> wrote:
>>>>>>>> Am Mittwoch, 30. September 2015, 16:42:05 schrieb Ulf Hansson:
>>>>>>>>> On 30 September 2015 at 16:07, Heiko Stuebner <heiko@sntech.de>
>> wrote:
>>>>>>>> The clock changes of course only touch internals of the phase-clocks,
>>>>>>>> so
>>>>>>>> should have no problem going through another tree.
>>>>>>>
>>>>>>> What happens if I take mmc and dt changes, wouldn't I need the clock
>>>>>>> patches as well?
>>>>>>
>>>>>> The API stays of course the same, only the degree to settings
>>>>>> translation gets optimized, so I guess in the worst case you would get
>>>>>> no good phase and thus fall back to non-highspeed modes - but the
>>>>>> system would stay running.
>>>>>>
>>>>>> But of course, if the clock maintainers could Ack the two clock patches
>>>>>> and
>>>>>> everything would stay together that would work even better :-)
>>>>>
>>>>> If Ulf doesn't want to take them we can apply them to clk tree.
>>>>> Otherwise, you can have my acked-by on the clk patches.
>>>>
>>>> I don't mind picking up the clock patches. So I consider this as an
>>>> ack for both patch 1 and patch2, thanks.
>>>>
>>>> Now, let's give Jaehoon some time to review the dw_mmc parts.
>>>
>>> I will check other patches on today, if it's ok, i will apply at my
>>> repository. Thanks for giving time! :)
>>
>> I think Ulf wanted to apply the whole series via the mmc tree directly, but I
>> guess that is between you two to decide ;-)
>
> If it will get Stephen's acked-by, i will pick up all after checked the dwmmc patches. how about?

Stephen already acked the clock patches from this thread.

Perhaps this time, it's easier if you ack the dw_mmc patches and I can
pick up the hole series? Does that work for you?

Kind regards
Uffe

[PATCH v2 3/8] mmc: core: Add mmc_regulator_set_vqmmc()

[Ulf Hansson]

On 2 October 2015 at 09:53, Jaehoon Chung <jh80.chung@samsung.com> wrote:
> On 10/02/2015 04:06 PM, Heiko St?bner wrote:
>> Am Freitag, 2. Oktober 2015, 11:52:00 schrieb Jaehoon Chung:
>>> On 10/02/2015 06:05 AM, Ulf Hansson wrote:
>>>> On 1 October 2015 at 19:35, Stephen Boyd <sboyd@codeaurora.org> wrote:
>>>>> On 10/01, Heiko St?bner wrote:
>>>>>> Am Donnerstag, 1. Oktober 2015, 11:54:24 schrieb Ulf Hansson:
>>>>>>> On 30 September 2015 at 16:55, Heiko St?bner <heiko@sntech.de> wrote:
>>>>>>>> Am Mittwoch, 30. September 2015, 16:42:05 schrieb Ulf Hansson:
>>>>>>>>> On 30 September 2015 at 16:07, Heiko Stuebner <heiko@sntech.de>
>> wrote:
>>>>>>>> The clock changes of course only touch internals of the phase-clocks,
>>>>>>>> so
>>>>>>>> should have no problem going through another tree.
>>>>>>>
>>>>>>> What happens if I take mmc and dt changes, wouldn't I need the clock
>>>>>>> patches as well?
>>>>>>
>>>>>> The API stays of course the same, only the degree to settings
>>>>>> translation gets optimized, so I guess in the worst case you would get
>>>>>> no good phase and thus fall back to non-highspeed modes - but the
>>>>>> system would stay running.
>>>>>>
>>>>>> But of course, if the clock maintainers could Ack the two clock patches
>>>>>> and
>>>>>> everything would stay together that would work even better :-)
>>>>>
>>>>> If Ulf doesn't want to take them we can apply them to clk tree.
>>>>> Otherwise, you can have my acked-by on the clk patches.
>>>>
>>>> I don't mind picking up the clock patches. So I consider this as an
>>>> ack for both patch 1 and patch2, thanks.
>>>>
>>>> Now, let's give Jaehoon some time to review the dw_mmc parts.
>>>
>>> I will check other patches on today, if it's ok, i will apply at my
>>> repository. Thanks for giving time! :)
>>
>> I think Ulf wanted to apply the whole series via the mmc tree directly, but I
>> guess that is between you two to decide ;-)
>
> If it will get Stephen's acked-by, i will pick up all after checked the dwmmc patches. how about?

Stephen already acked the clock patches from this thread.

Perhaps this time, it's easier if you ack the dw_mmc patches and I can
pick up the hole series? Does that work for you?

Kind regards
Uffe

Re: [PATCH v2 3/8] mmc: core: Add mmc_regulator_set_vqmmc()

[Jaehoon Chung]

On 10/02/2015 05:43 PM, Ulf Hansson wrote:
> On 2 October 2015 at 09:53, Jaehoon Chung <jh80.chung@samsung.com> wrote:
>> On 10/02/2015 04:06 PM, Heiko St=C3=BCbner wrote:
>>> Am Freitag, 2. Oktober 2015, 11:52:00 schrieb Jaehoon Chung:
>>>> On 10/02/2015 06:05 AM, Ulf Hansson wrote:
>>>>> On 1 October 2015 at 19:35, Stephen Boyd <sboyd@codeaurora.org> wrote:
>>>>>> On 10/01, Heiko St=C3=BCbner wrote:
>>>>>>> Am Donnerstag, 1. Oktober 2015, 11:54:24 schrieb Ulf Hansson:
>>>>>>>> On 30 September 2015 at 16:55, Heiko St=C3=BCbner <heiko@sntech.de> wrote:
>>>>>>>>> Am Mittwoch, 30. September 2015, 16:42:05 schrieb Ulf Hansson:
>>>>>>>>>> On 30 September 2015 at 16:07, Heiko Stuebner <heiko@sntech.de>
>>> wrote:
>>>>>>>>> The clock changes of course only touch internals of the phase-clocks,
>>>>>>>>> so
>>>>>>>>> should have no problem going through another tree.
>>>>>>>>
>>>>>>>> What happens if I take mmc and dt changes, wouldn't I need the clock
>>>>>>>> patches as well=3F
>>>>>>>
>>>>>>> The API stays of course the same, only the degree to settings
>>>>>>> translation gets optimized, so I guess in the worst case you would get
>>>>>>> no good phase and thus fall back to non-highspeed modes - but the
>>>>>>> system would stay running.
>>>>>>>
>>>>>>> But of course, if the clock maintainers could Ack the two clock patches
>>>>>>> and
>>>>>>> everything would stay together that would work even better :-)
>>>>>>
>>>>>> If Ulf doesn't want to take them we can apply them to clk tree.
>>>>>> Otherwise, you can have my acked-by on the clk patches.
>>>>>
>>>>> I don't mind picking up the clock patches. So I consider this as an
>>>>> ack for both patch 1 and patch2, thanks.
>>>>>
>>>>> Now, let's give Jaehoon some time to review the dw_mmc parts.
>>>>
>>>> I will check other patches on today, if it's ok, i will apply at my
>>>> repository. Thanks for giving time! :)
>>>
>>> I think Ulf wanted to apply the whole series via the mmc tree directly, but I
>>> guess that is between you two to decide ;-)
>>
>> If it will get Stephen's acked-by, i will pick up all after checked the dwmmc patches. how about=3F
> 
> Stephen already acked the clock patches from this thread.
> 
> Perhaps this time, it's easier if you ack the dw_mmc patches and I can
> pick up the hole series=3F Does that work for you=3F

I doesn't review dwmmc patches (Sorry), could you give me more time=3F
If you're ok, i will notice to you...then you can pick up all. :)

Best Regards,
Jaehoon Chung

> 
> Kind regards
> Uffe
> 

Re: [PATCH v2 3/8] mmc: core: Add mmc_regulator_set_vqmmc()

[Jaehoon Chung]

On 10/02/2015 05:43 PM, Ulf Hansson wrote:
> On 2 October 2015 at 09:53, Jaehoon Chung <jh80.chung@samsung.com> wrote:
>> On 10/02/2015 04:06 PM, Heiko Stübner wrote:
>>> Am Freitag, 2. Oktober 2015, 11:52:00 schrieb Jaehoon Chung:
>>>> On 10/02/2015 06:05 AM, Ulf Hansson wrote:
>>>>> On 1 October 2015 at 19:35, Stephen Boyd <sboyd@codeaurora.org> wrote:
>>>>>> On 10/01, Heiko Stübner wrote:
>>>>>>> Am Donnerstag, 1. Oktober 2015, 11:54:24 schrieb Ulf Hansson:
>>>>>>>> On 30 September 2015 at 16:55, Heiko Stübner <heiko@sntech.de> wrote:
>>>>>>>>> Am Mittwoch, 30. September 2015, 16:42:05 schrieb Ulf Hansson:
>>>>>>>>>> On 30 September 2015 at 16:07, Heiko Stuebner <heiko@sntech.de>
>>> wrote:
>>>>>>>>> The clock changes of course only touch internals of the phase-clocks,
>>>>>>>>> so
>>>>>>>>> should have no problem going through another tree.
>>>>>>>>
>>>>>>>> What happens if I take mmc and dt changes, wouldn't I need the clock
>>>>>>>> patches as well?
>>>>>>>
>>>>>>> The API stays of course the same, only the degree to settings
>>>>>>> translation gets optimized, so I guess in the worst case you would get
>>>>>>> no good phase and thus fall back to non-highspeed modes - but the
>>>>>>> system would stay running.
>>>>>>>
>>>>>>> But of course, if the clock maintainers could Ack the two clock patches
>>>>>>> and
>>>>>>> everything would stay together that would work even better :-)
>>>>>>
>>>>>> If Ulf doesn't want to take them we can apply them to clk tree.
>>>>>> Otherwise, you can have my acked-by on the clk patches.
>>>>>
>>>>> I don't mind picking up the clock patches. So I consider this as an
>>>>> ack for both patch 1 and patch2, thanks.
>>>>>
>>>>> Now, let's give Jaehoon some time to review the dw_mmc parts.
>>>>
>>>> I will check other patches on today, if it's ok, i will apply at my
>>>> repository. Thanks for giving time! :)
>>>
>>> I think Ulf wanted to apply the whole series via the mmc tree directly, but I
>>> guess that is between you two to decide ;-)
>>
>> If it will get Stephen's acked-by, i will pick up all after checked the dwmmc patches. how about?
> 
> Stephen already acked the clock patches from this thread.
> 
> Perhaps this time, it's easier if you ack the dw_mmc patches and I can
> pick up the hole series? Does that work for you?

I doesn't review dwmmc patches (Sorry), could you give me more time?
If you're ok, i will notice to you...then you can pick up all. :)

Best Regards,
Jaehoon Chung

> 
> Kind regards
> Uffe
> 

[PATCH v2 3/8] mmc: core: Add mmc_regulator_set_vqmmc()

[Jaehoon Chung]

On 10/02/2015 05:43 PM, Ulf Hansson wrote:
> On 2 October 2015 at 09:53, Jaehoon Chung <jh80.chung@samsung.com> wrote:
>> On 10/02/2015 04:06 PM, Heiko St?bner wrote:
>>> Am Freitag, 2. Oktober 2015, 11:52:00 schrieb Jaehoon Chung:
>>>> On 10/02/2015 06:05 AM, Ulf Hansson wrote:
>>>>> On 1 October 2015 at 19:35, Stephen Boyd <sboyd@codeaurora.org> wrote:
>>>>>> On 10/01, Heiko St?bner wrote:
>>>>>>> Am Donnerstag, 1. Oktober 2015, 11:54:24 schrieb Ulf Hansson:
>>>>>>>> On 30 September 2015 at 16:55, Heiko St?bner <heiko@sntech.de> wrote:
>>>>>>>>> Am Mittwoch, 30. September 2015, 16:42:05 schrieb Ulf Hansson:
>>>>>>>>>> On 30 September 2015 at 16:07, Heiko Stuebner <heiko@sntech.de>
>>> wrote:
>>>>>>>>> The clock changes of course only touch internals of the phase-clocks,
>>>>>>>>> so
>>>>>>>>> should have no problem going through another tree.
>>>>>>>>
>>>>>>>> What happens if I take mmc and dt changes, wouldn't I need the clock
>>>>>>>> patches as well?
>>>>>>>
>>>>>>> The API stays of course the same, only the degree to settings
>>>>>>> translation gets optimized, so I guess in the worst case you would get
>>>>>>> no good phase and thus fall back to non-highspeed modes - but the
>>>>>>> system would stay running.
>>>>>>>
>>>>>>> But of course, if the clock maintainers could Ack the two clock patches
>>>>>>> and
>>>>>>> everything would stay together that would work even better :-)
>>>>>>
>>>>>> If Ulf doesn't want to take them we can apply them to clk tree.
>>>>>> Otherwise, you can have my acked-by on the clk patches.
>>>>>
>>>>> I don't mind picking up the clock patches. So I consider this as an
>>>>> ack for both patch 1 and patch2, thanks.
>>>>>
>>>>> Now, let's give Jaehoon some time to review the dw_mmc parts.
>>>>
>>>> I will check other patches on today, if it's ok, i will apply at my
>>>> repository. Thanks for giving time! :)
>>>
>>> I think Ulf wanted to apply the whole series via the mmc tree directly, but I
>>> guess that is between you two to decide ;-)
>>
>> If it will get Stephen's acked-by, i will pick up all after checked the dwmmc patches. how about?
> 
> Stephen already acked the clock patches from this thread.
> 
> Perhaps this time, it's easier if you ack the dw_mmc patches and I can
> pick up the hole series? Does that work for you?

I doesn't review dwmmc patches (Sorry), could you give me more time?
If you're ok, i will notice to you...then you can pick up all. :)

Best Regards,
Jaehoon Chung

> 
> Kind regards
> Uffe
> 

Re: [PATCH v2 3/8] mmc: core: Add mmc_regulator_set_vqmmc()

[Ulf Hansson]

[...]

>>>>>> Now, let's give Jaehoon some time to review the dw_mmc parts.
>>>>>
>>>>> I will check other patches on today, if it's ok, i will apply at my
>>>>> repository. Thanks for giving time! :)
>>>>
>>>> I think Ulf wanted to apply the whole series via the mmc tree directly, but I
>>>> guess that is between you two to decide ;-)
>>>
>>> If it will get Stephen's acked-by, i will pick up all after checked the dwmmc patches. how about?
>>
>> Stephen already acked the clock patches from this thread.
>>
>> Perhaps this time, it's easier if you ack the dw_mmc patches and I can
>> pick up the hole series? Does that work for you?
>
> I doesn't review dwmmc patches (Sorry), could you give me more time?
> If you're ok, i will notice to you...then you can pick up all. :)

Sure, no worries. Take your time!

Kind regards
Uffe

[PATCH v2 3/8] mmc: core: Add mmc_regulator_set_vqmmc()

[Ulf Hansson]

[...]

>>>>>> Now, let's give Jaehoon some time to review the dw_mmc parts.
>>>>>
>>>>> I will check other patches on today, if it's ok, i will apply at my
>>>>> repository. Thanks for giving time! :)
>>>>
>>>> I think Ulf wanted to apply the whole series via the mmc tree directly, but I
>>>> guess that is between you two to decide ;-)
>>>
>>> If it will get Stephen's acked-by, i will pick up all after checked the dwmmc patches. how about?
>>
>> Stephen already acked the clock patches from this thread.
>>
>> Perhaps this time, it's easier if you ack the dw_mmc patches and I can
>> pick up the hole series? Does that work for you?
>
> I doesn't review dwmmc patches (Sorry), could you give me more time?
> If you're ok, i will notice to you...then you can pick up all. :)

Sure, no worries. Take your time!

Kind regards
Uffe

Re: [PATCH v2 4/8] mmc: dw_mmc: Use mmc_regulator_set_vqmmc in start_signal_voltage_switch

[Jaehoon Chung]

Hi, Heiko.

Acked-by: Jaehoon Chung <jh80.chung@samsung.com>

Best Regards,
Jaehoon Chung


On 09/30/2015 11:07 PM, Heiko Stuebner wrote:
> From: Douglas Anderson <dianders@chromium.org>
> 
> We've introduced a new helper in the MMC core:
> mmc_regulator_set_vqmmc().  Let's use this in dw_mmc.  Using this new
> helper has some advantages:
> 
> 1. We get the mmc_regulator_set_vqmmc() behavior of trying to match
>    VQMMC and VMMC when the signal voltage is 3.3V.  This ensures max
>    compatibility.
> 
> 2. We get rid of a few more warnings when probing unsupported
>    voltages.
> 
> 3. We get rid of some non-dw_mmc specific code in dw_mmc.
> 
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> ---
>  drivers/mmc/host/dw_mmc.c | 17 ++++++-----------
>  1 file changed, 6 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index fcbf552..b1b7e7f 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -1279,7 +1279,6 @@ static int dw_mci_switch_voltage(struct mmc_host *mmc, struct mmc_ios *ios)
>  	const struct dw_mci_drv_data *drv_data = host->drv_data;
>  	u32 uhs;
>  	u32 v18 = SDMMC_UHS_18V << slot->id;
> -	int min_uv, max_uv;
>  	int ret;
>  
>  	if (drv_data && drv_data->switch_voltage)
> @@ -1291,22 +1290,18 @@ static int dw_mci_switch_voltage(struct mmc_host *mmc, struct mmc_ios *ios)
>  	 * does no harm but you need to set the regulator directly.  Try both.
>  	 */
>  	uhs = mci_readl(host, UHS_REG);
> -	if (ios->signal_voltage == MMC_SIGNAL_VOLTAGE_330) {
> -		min_uv = 2700000;
> -		max_uv = 3600000;
> +	if (ios->signal_voltage == MMC_SIGNAL_VOLTAGE_330)
>  		uhs &= ~v18;
> -	} else {
> -		min_uv = 1700000;
> -		max_uv = 1950000;
> +	else
>  		uhs |= v18;
> -	}
> +
>  	if (!IS_ERR(mmc->supply.vqmmc)) {
> -		ret = regulator_set_voltage(mmc->supply.vqmmc, min_uv, max_uv);
> +		ret = mmc_regulator_set_vqmmc(mmc, ios);
>  
>  		if (ret) {
>  			dev_dbg(&mmc->class_dev,
> -					 "Regulator set error %d: %d - %d\n",
> -					 ret, min_uv, max_uv);
> +					 "Regulator set error %d - %s V\n",
> +					 ret, uhs & v18 ? "1.8" : "3.3");
>  			return ret;
>  		}
>  	}
> 

[PATCH v2 4/8] mmc: dw_mmc: Use mmc_regulator_set_vqmmc in start_signal_voltage_switch

[Jaehoon Chung]

Hi, Heiko.

Acked-by: Jaehoon Chung <jh80.chung@samsung.com>

Best Regards,
Jaehoon Chung


On 09/30/2015 11:07 PM, Heiko Stuebner wrote:
> From: Douglas Anderson <dianders@chromium.org>
> 
> We've introduced a new helper in the MMC core:
> mmc_regulator_set_vqmmc().  Let's use this in dw_mmc.  Using this new
> helper has some advantages:
> 
> 1. We get the mmc_regulator_set_vqmmc() behavior of trying to match
>    VQMMC and VMMC when the signal voltage is 3.3V.  This ensures max
>    compatibility.
> 
> 2. We get rid of a few more warnings when probing unsupported
>    voltages.
> 
> 3. We get rid of some non-dw_mmc specific code in dw_mmc.
> 
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> ---
>  drivers/mmc/host/dw_mmc.c | 17 ++++++-----------
>  1 file changed, 6 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index fcbf552..b1b7e7f 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -1279,7 +1279,6 @@ static int dw_mci_switch_voltage(struct mmc_host *mmc, struct mmc_ios *ios)
>  	const struct dw_mci_drv_data *drv_data = host->drv_data;
>  	u32 uhs;
>  	u32 v18 = SDMMC_UHS_18V << slot->id;
> -	int min_uv, max_uv;
>  	int ret;
>  
>  	if (drv_data && drv_data->switch_voltage)
> @@ -1291,22 +1290,18 @@ static int dw_mci_switch_voltage(struct mmc_host *mmc, struct mmc_ios *ios)
>  	 * does no harm but you need to set the regulator directly.  Try both.
>  	 */
>  	uhs = mci_readl(host, UHS_REG);
> -	if (ios->signal_voltage == MMC_SIGNAL_VOLTAGE_330) {
> -		min_uv = 2700000;
> -		max_uv = 3600000;
> +	if (ios->signal_voltage == MMC_SIGNAL_VOLTAGE_330)
>  		uhs &= ~v18;
> -	} else {
> -		min_uv = 1700000;
> -		max_uv = 1950000;
> +	else
>  		uhs |= v18;
> -	}
> +
>  	if (!IS_ERR(mmc->supply.vqmmc)) {
> -		ret = regulator_set_voltage(mmc->supply.vqmmc, min_uv, max_uv);
> +		ret = mmc_regulator_set_vqmmc(mmc, ios);
>  
>  		if (ret) {
>  			dev_dbg(&mmc->class_dev,
> -					 "Regulator set error %d: %d - %d\n",
> -					 ret, min_uv, max_uv);
> +					 "Regulator set error %d - %s V\n",
> +					 ret, uhs & v18 ? "1.8" : "3.3");
>  			return ret;
>  		}
>  	}
> 

Re: [PATCH v2 5/8] mmc: dw_mmc-rockchip: dt-binding: Add tuning related things

[Jaehoon Chung]

Hi, Heiko.

Acked-by: Jaehoon Chung <jh80.chung@samsung.com>

Best Regards,
Jaehoon Chung


On 09/30/2015 11:07 PM, Heiko Stuebner wrote:
> From: Alexandru M Stan <amstan@chromium.org>
> 
> Add ciu_drive, ciu_sample clocks and default-sample-phase. This will later
> be used by tuning code.
> 
> We do not touch ciu_drive (and by extension define default-drive-phase).
> Drive phase is mostly used to define minimum hold times, while one could
> write some code to determine what phase meets the minimum hold time
> (ex 10 degrees) this will not work with the current clock phase framework
> (which floors angles, so we'll get 0 deg, and there's no way to know what
> resolution the floors happen at). We assume that the default drive angles
> set by the hardware are good enough.
> 
> Signed-off-by: Alexandru M Stan <amstan@chromium.org>
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> ---
>  Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt b/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt
> index c327c2d..3dc13b6 100644
> --- a/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt
> +++ b/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt
> @@ -14,6 +14,19 @@ Required Properties:
>  							before RK3288
>  	- "rockchip,rk3288-dw-mshc": for Rockchip RK3288
>  
> +Optional Properties:
> +* clocks: from common clock binding: if ciu_drive and ciu_sample are
> +  specified in clock-names, should contain handles to these clocks.
> +
> +* clock-names: Apart from the clock-names described in synopsys-dw-mshc.txt
> +  two more clocks "ciu-drive" and "ciu-sample" are supported. They are used
> +  to control the clock phases, "ciu-sample" is required for tuning high-
> +  speed modes.
> +
> +* rockchip,default-sample-phase: The default phase to set ciu_sample at
> +  probing, low speeds or in case where all phases work at tuning time.
> +  If not specified 0 deg will be used.
> +
>  Example:
>  
>  	rkdwmmc0@12200000 {
> 

[PATCH v2 5/8] mmc: dw_mmc-rockchip: dt-binding: Add tuning related things

[Jaehoon Chung]

Hi, Heiko.

Acked-by: Jaehoon Chung <jh80.chung@samsung.com>

Best Regards,
Jaehoon Chung


On 09/30/2015 11:07 PM, Heiko Stuebner wrote:
> From: Alexandru M Stan <amstan@chromium.org>
> 
> Add ciu_drive, ciu_sample clocks and default-sample-phase. This will later
> be used by tuning code.
> 
> We do not touch ciu_drive (and by extension define default-drive-phase).
> Drive phase is mostly used to define minimum hold times, while one could
> write some code to determine what phase meets the minimum hold time
> (ex 10 degrees) this will not work with the current clock phase framework
> (which floors angles, so we'll get 0 deg, and there's no way to know what
> resolution the floors happen at). We assume that the default drive angles
> set by the hardware are good enough.
> 
> Signed-off-by: Alexandru M Stan <amstan@chromium.org>
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> ---
>  Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt b/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt
> index c327c2d..3dc13b6 100644
> --- a/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt
> +++ b/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt
> @@ -14,6 +14,19 @@ Required Properties:
>  							before RK3288
>  	- "rockchip,rk3288-dw-mshc": for Rockchip RK3288
>  
> +Optional Properties:
> +* clocks: from common clock binding: if ciu_drive and ciu_sample are
> +  specified in clock-names, should contain handles to these clocks.
> +
> +* clock-names: Apart from the clock-names described in synopsys-dw-mshc.txt
> +  two more clocks "ciu-drive" and "ciu-sample" are supported. They are used
> +  to control the clock phases, "ciu-sample" is required for tuning high-
> +  speed modes.
> +
> +* rockchip,default-sample-phase: The default phase to set ciu_sample at
> +  probing, low speeds or in case where all phases work at tuning time.
> +  If not specified 0 deg will be used.
> +
>  Example:
>  
>  	rkdwmmc0 at 12200000 {
> 

Re: [PATCH v2 6/8] mmc: dw_mmc-rockchip: MMC tuning with the clock phase framework

[Jaehoon Chung]

Hi, Heiko.

Acked-by: Jaehoon Chung <jh80.chung@samsung.com>

Best Regards,
Jaehoon Chung


On 09/30/2015 11:07 PM, Heiko Stuebner wrote:
> From: Alexandru M Stan <amstan@chromium.org>
> 
> This algorithm will try 1 degree increments, since there's no way to tell
> what resolution the underlying phase code uses. As an added bonus, doing
> many tunings yields better results since some tests are run more than once
> (ex: if the underlying driver uses 45 degree increments, the tuning code
> will try the same angle more than once).
> 
> It will then construct a list of good phase ranges (even ranges that cross
> 360/0), will pick the biggest range then it will set the sample_clk to the
> middle of that range.
> 
> We do not touch ciu_drive (and by extension define default-drive-phase).
> Drive phase is mostly used to define minimum hold times, while one could
> write some code to determine what phase meets the minimum hold time (ex 10
> degrees) this will not work with the current clock phase framework (which
> floors angles, so we'll get 0 deg, and there's no way to know what
> resolution the floors happen at). We assume that the default drive angles
> set by the hardware are good enough.
> 
> If a device has device specific code (like exynos) then that will still
> take precedence, otherwise this new code will execute. If the device wants
> to tune, but has no sample_clk defined we'll return EIO with an error
> message.
> 
> Signed-off-by: Alexandru M Stan <amstan@chromium.org>
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> ---
>  drivers/mmc/host/dw_mmc-rockchip.c | 162 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 162 insertions(+)
> 
> diff --git a/drivers/mmc/host/dw_mmc-rockchip.c b/drivers/mmc/host/dw_mmc-rockchip.c
> index bc76aa2..4b3650f 100644
> --- a/drivers/mmc/host/dw_mmc-rockchip.c
> +++ b/drivers/mmc/host/dw_mmc-rockchip.c
> @@ -13,12 +13,19 @@
>  #include <linux/mmc/host.h>
>  #include <linux/mmc/dw_mmc.h>
>  #include <linux/of_address.h>
> +#include <linux/slab.h>
>  
>  #include "dw_mmc.h"
>  #include "dw_mmc-pltfm.h"
>  
>  #define RK3288_CLKGEN_DIV       2
>  
> +struct dw_mci_rockchip_priv_data {
> +	struct clk		*drv_clk;
> +	struct clk		*sample_clk;
> +	int			default_sample_phase;
> +};
> +
>  static void dw_mci_rockchip_prepare_command(struct dw_mci *host, u32 *cmdr)
>  {
>  	*cmdr |= SDMMC_CMD_USE_HOLD_REG;
> @@ -33,6 +40,7 @@ static int dw_mci_rk3288_setup_clock(struct dw_mci *host)
>  
>  static void dw_mci_rk3288_set_ios(struct dw_mci *host, struct mmc_ios *ios)
>  {
> +	struct dw_mci_rockchip_priv_data *priv = host->priv;
>  	int ret;
>  	unsigned int cclkin;
>  	u32 bus_hz;
> @@ -66,6 +74,158 @@ static void dw_mci_rk3288_set_ios(struct dw_mci *host, struct mmc_ios *ios)
>  		/* force dw_mci_setup_bus() */
>  		host->current_speed = 0;
>  	}
> +
> +	/* Make sure we use phases which we can enumerate with */
> +	if (!IS_ERR(priv->sample_clk))
> +		clk_set_phase(priv->sample_clk, priv->default_sample_phase);
> +}
> +
> +#define NUM_PHASES			360
> +#define TUNING_ITERATION_TO_PHASE(i)	(DIV_ROUND_UP((i) * 360, NUM_PHASES))
> +
> +static int dw_mci_rk3288_execute_tuning(struct dw_mci_slot *slot)
> +{
> +	struct dw_mci *host = slot->host;
> +	struct dw_mci_rockchip_priv_data *priv = host->priv;
> +	struct mmc_host *mmc = slot->mmc;
> +	int ret = 0;
> +	int i;
> +	bool v, prev_v = 0, first_v;
> +	struct range_t {
> +		int start;
> +		int end; /* inclusive */
> +	};
> +	struct range_t *ranges;
> +	unsigned int range_count = 0;
> +	int longest_range_len = -1;
> +	int longest_range = -1;
> +	int middle_phase;
> +
> +	if (IS_ERR(priv->sample_clk)) {
> +		dev_err(host->dev, "Tuning clock (sample_clk) not defined.\n");
> +		return -EIO;
> +	}
> +
> +	ranges = kmalloc_array(NUM_PHASES / 2 + 1, sizeof(*ranges), GFP_KERNEL);
> +	if (!ranges)
> +		return -ENOMEM;
> +
> +	/* Try each phase and extract good ranges */
> +	for (i = 0; i < NUM_PHASES; ) {
> +		clk_set_phase(priv->sample_clk, TUNING_ITERATION_TO_PHASE(i));
> +
> +		v = !mmc_send_tuning(mmc);
> +
> +		if (i == 0)
> +			first_v = v;
> +
> +		if ((!prev_v) && v) {
> +			range_count++;
> +			ranges[range_count-1].start = i;
> +		}
> +		if (v) {
> +			ranges[range_count-1].end = i;
> +			i++;
> +		} else if (i == NUM_PHASES - 1) {
> +			/* No extra skipping rules if we're at the end */
> +			i++;
> +		} else {
> +			/*
> +			 * No need to check too close to an invalid
> +			 * one since testing bad phases is slow.  Skip
> +			 * 20 degrees.
> +			 */
> +			i += DIV_ROUND_UP(20 * NUM_PHASES, 360);
> +
> +			/* Always test the last one */
> +			if (i >= NUM_PHASES)
> +				i = NUM_PHASES - 1;
> +		}
> +
> +		prev_v = v;
> +	}
> +
> +	if (range_count == 0) {
> +		dev_warn(host->dev, "All phases bad!");
> +		ret = -EIO;
> +		goto free;
> +	}
> +
> +	/* wrap around case, merge the end points */
> +	if ((range_count > 1) && first_v && v) {
> +		ranges[0].start = ranges[range_count-1].start;
> +		range_count--;
> +	}
> +
> +	if (ranges[0].start == 0 && ranges[0].end == NUM_PHASES - 1) {
> +		clk_set_phase(priv->sample_clk, priv->default_sample_phase);
> +		dev_info(host->dev, "All phases work, using default phase %d.",
> +			 priv->default_sample_phase);
> +		goto free;
> +	}
> +
> +	/* Find the longest range */
> +	for (i = 0; i < range_count; i++) {
> +		int len = (ranges[i].end - ranges[i].start + 1);
> +
> +		if (len < 0)
> +			len += NUM_PHASES;
> +
> +		if (longest_range_len < len) {
> +			longest_range_len = len;
> +			longest_range = i;
> +		}
> +
> +		dev_dbg(host->dev, "Good phase range %d-%d (%d len)\n",
> +			TUNING_ITERATION_TO_PHASE(ranges[i].start),
> +			TUNING_ITERATION_TO_PHASE(ranges[i].end),
> +			len
> +		);
> +	}
> +
> +	dev_dbg(host->dev, "Best phase range %d-%d (%d len)\n",
> +		TUNING_ITERATION_TO_PHASE(ranges[longest_range].start),
> +		TUNING_ITERATION_TO_PHASE(ranges[longest_range].end),
> +		longest_range_len
> +	);
> +
> +	middle_phase = ranges[longest_range].start + longest_range_len / 2;
> +	middle_phase %= NUM_PHASES;
> +	dev_info(host->dev, "Successfully tuned phase to %d\n",
> +		 TUNING_ITERATION_TO_PHASE(middle_phase));
> +
> +	clk_set_phase(priv->sample_clk,
> +		      TUNING_ITERATION_TO_PHASE(middle_phase));
> +
> +free:
> +	kfree(ranges);
> +	return ret;
> +}
> +
> +static int dw_mci_rk3288_parse_dt(struct dw_mci *host)
> +{
> +	struct device_node *np = host->dev->of_node;
> +	struct dw_mci_rockchip_priv_data *priv;
> +
> +	priv = devm_kzalloc(host->dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	if (of_property_read_u32(np, "rockchip,default-sample-phase",
> +					&priv->default_sample_phase))
> +		priv->default_sample_phase = 0;
> +
> +	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");
> +
> +	priv->sample_clk = devm_clk_get(host->dev, "ciu-sample");
> +	if (IS_ERR(priv->sample_clk))
> +		dev_dbg(host->dev, "ciu_sample not available\n");
> +
> +	host->priv = priv;
> +
> +	return 0;
>  }
>  
>  static int dw_mci_rockchip_init(struct dw_mci *host)
> @@ -95,6 +255,8 @@ static const struct dw_mci_drv_data rk3288_drv_data = {
>  	.caps			= dw_mci_rk3288_dwmmc_caps,
>  	.prepare_command        = dw_mci_rockchip_prepare_command,
>  	.set_ios		= dw_mci_rk3288_set_ios,
> +	.execute_tuning		= dw_mci_rk3288_execute_tuning,
> +	.parse_dt		= dw_mci_rk3288_parse_dt,
>  	.setup_clock    = dw_mci_rk3288_setup_clock,
>  	.init			= dw_mci_rockchip_init,
>  };
> 

[PATCH v2 6/8] mmc: dw_mmc-rockchip: MMC tuning with the clock phase framework

[Jaehoon Chung]

Hi, Heiko.

Acked-by: Jaehoon Chung <jh80.chung@samsung.com>

Best Regards,
Jaehoon Chung


On 09/30/2015 11:07 PM, Heiko Stuebner wrote:
> From: Alexandru M Stan <amstan@chromium.org>
> 
> This algorithm will try 1 degree increments, since there's no way to tell
> what resolution the underlying phase code uses. As an added bonus, doing
> many tunings yields better results since some tests are run more than once
> (ex: if the underlying driver uses 45 degree increments, the tuning code
> will try the same angle more than once).
> 
> It will then construct a list of good phase ranges (even ranges that cross
> 360/0), will pick the biggest range then it will set the sample_clk to the
> middle of that range.
> 
> We do not touch ciu_drive (and by extension define default-drive-phase).
> Drive phase is mostly used to define minimum hold times, while one could
> write some code to determine what phase meets the minimum hold time (ex 10
> degrees) this will not work with the current clock phase framework (which
> floors angles, so we'll get 0 deg, and there's no way to know what
> resolution the floors happen at). We assume that the default drive angles
> set by the hardware are good enough.
> 
> If a device has device specific code (like exynos) then that will still
> take precedence, otherwise this new code will execute. If the device wants
> to tune, but has no sample_clk defined we'll return EIO with an error
> message.
> 
> Signed-off-by: Alexandru M Stan <amstan@chromium.org>
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> ---
>  drivers/mmc/host/dw_mmc-rockchip.c | 162 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 162 insertions(+)
> 
> diff --git a/drivers/mmc/host/dw_mmc-rockchip.c b/drivers/mmc/host/dw_mmc-rockchip.c
> index bc76aa2..4b3650f 100644
> --- a/drivers/mmc/host/dw_mmc-rockchip.c
> +++ b/drivers/mmc/host/dw_mmc-rockchip.c
> @@ -13,12 +13,19 @@
>  #include <linux/mmc/host.h>
>  #include <linux/mmc/dw_mmc.h>
>  #include <linux/of_address.h>
> +#include <linux/slab.h>
>  
>  #include "dw_mmc.h"
>  #include "dw_mmc-pltfm.h"
>  
>  #define RK3288_CLKGEN_DIV       2
>  
> +struct dw_mci_rockchip_priv_data {
> +	struct clk		*drv_clk;
> +	struct clk		*sample_clk;
> +	int			default_sample_phase;
> +};
> +
>  static void dw_mci_rockchip_prepare_command(struct dw_mci *host, u32 *cmdr)
>  {
>  	*cmdr |= SDMMC_CMD_USE_HOLD_REG;
> @@ -33,6 +40,7 @@ static int dw_mci_rk3288_setup_clock(struct dw_mci *host)
>  
>  static void dw_mci_rk3288_set_ios(struct dw_mci *host, struct mmc_ios *ios)
>  {
> +	struct dw_mci_rockchip_priv_data *priv = host->priv;
>  	int ret;
>  	unsigned int cclkin;
>  	u32 bus_hz;
> @@ -66,6 +74,158 @@ static void dw_mci_rk3288_set_ios(struct dw_mci *host, struct mmc_ios *ios)
>  		/* force dw_mci_setup_bus() */
>  		host->current_speed = 0;
>  	}
> +
> +	/* Make sure we use phases which we can enumerate with */
> +	if (!IS_ERR(priv->sample_clk))
> +		clk_set_phase(priv->sample_clk, priv->default_sample_phase);
> +}
> +
> +#define NUM_PHASES			360
> +#define TUNING_ITERATION_TO_PHASE(i)	(DIV_ROUND_UP((i) * 360, NUM_PHASES))
> +
> +static int dw_mci_rk3288_execute_tuning(struct dw_mci_slot *slot)
> +{
> +	struct dw_mci *host = slot->host;
> +	struct dw_mci_rockchip_priv_data *priv = host->priv;
> +	struct mmc_host *mmc = slot->mmc;
> +	int ret = 0;
> +	int i;
> +	bool v, prev_v = 0, first_v;
> +	struct range_t {
> +		int start;
> +		int end; /* inclusive */
> +	};
> +	struct range_t *ranges;
> +	unsigned int range_count = 0;
> +	int longest_range_len = -1;
> +	int longest_range = -1;
> +	int middle_phase;
> +
> +	if (IS_ERR(priv->sample_clk)) {
> +		dev_err(host->dev, "Tuning clock (sample_clk) not defined.\n");
> +		return -EIO;
> +	}
> +
> +	ranges = kmalloc_array(NUM_PHASES / 2 + 1, sizeof(*ranges), GFP_KERNEL);
> +	if (!ranges)
> +		return -ENOMEM;
> +
> +	/* Try each phase and extract good ranges */
> +	for (i = 0; i < NUM_PHASES; ) {
> +		clk_set_phase(priv->sample_clk, TUNING_ITERATION_TO_PHASE(i));
> +
> +		v = !mmc_send_tuning(mmc);
> +
> +		if (i == 0)
> +			first_v = v;
> +
> +		if ((!prev_v) && v) {
> +			range_count++;
> +			ranges[range_count-1].start = i;
> +		}
> +		if (v) {
> +			ranges[range_count-1].end = i;
> +			i++;
> +		} else if (i == NUM_PHASES - 1) {
> +			/* No extra skipping rules if we're at the end */
> +			i++;
> +		} else {
> +			/*
> +			 * No need to check too close to an invalid
> +			 * one since testing bad phases is slow.  Skip
> +			 * 20 degrees.
> +			 */
> +			i += DIV_ROUND_UP(20 * NUM_PHASES, 360);
> +
> +			/* Always test the last one */
> +			if (i >= NUM_PHASES)
> +				i = NUM_PHASES - 1;
> +		}
> +
> +		prev_v = v;
> +	}
> +
> +	if (range_count == 0) {
> +		dev_warn(host->dev, "All phases bad!");
> +		ret = -EIO;
> +		goto free;
> +	}
> +
> +	/* wrap around case, merge the end points */
> +	if ((range_count > 1) && first_v && v) {
> +		ranges[0].start = ranges[range_count-1].start;
> +		range_count--;
> +	}
> +
> +	if (ranges[0].start == 0 && ranges[0].end == NUM_PHASES - 1) {
> +		clk_set_phase(priv->sample_clk, priv->default_sample_phase);
> +		dev_info(host->dev, "All phases work, using default phase %d.",
> +			 priv->default_sample_phase);
> +		goto free;
> +	}
> +
> +	/* Find the longest range */
> +	for (i = 0; i < range_count; i++) {
> +		int len = (ranges[i].end - ranges[i].start + 1);
> +
> +		if (len < 0)
> +			len += NUM_PHASES;
> +
> +		if (longest_range_len < len) {
> +			longest_range_len = len;
> +			longest_range = i;
> +		}
> +
> +		dev_dbg(host->dev, "Good phase range %d-%d (%d len)\n",
> +			TUNING_ITERATION_TO_PHASE(ranges[i].start),
> +			TUNING_ITERATION_TO_PHASE(ranges[i].end),
> +			len
> +		);
> +	}
> +
> +	dev_dbg(host->dev, "Best phase range %d-%d (%d len)\n",
> +		TUNING_ITERATION_TO_PHASE(ranges[longest_range].start),
> +		TUNING_ITERATION_TO_PHASE(ranges[longest_range].end),
> +		longest_range_len
> +	);
> +
> +	middle_phase = ranges[longest_range].start + longest_range_len / 2;
> +	middle_phase %= NUM_PHASES;
> +	dev_info(host->dev, "Successfully tuned phase to %d\n",
> +		 TUNING_ITERATION_TO_PHASE(middle_phase));
> +
> +	clk_set_phase(priv->sample_clk,
> +		      TUNING_ITERATION_TO_PHASE(middle_phase));
> +
> +free:
> +	kfree(ranges);
> +	return ret;
> +}
> +
> +static int dw_mci_rk3288_parse_dt(struct dw_mci *host)
> +{
> +	struct device_node *np = host->dev->of_node;
> +	struct dw_mci_rockchip_priv_data *priv;
> +
> +	priv = devm_kzalloc(host->dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	if (of_property_read_u32(np, "rockchip,default-sample-phase",
> +					&priv->default_sample_phase))
> +		priv->default_sample_phase = 0;
> +
> +	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");
> +
> +	priv->sample_clk = devm_clk_get(host->dev, "ciu-sample");
> +	if (IS_ERR(priv->sample_clk))
> +		dev_dbg(host->dev, "ciu_sample not available\n");
> +
> +	host->priv = priv;
> +
> +	return 0;
>  }
>  
>  static int dw_mci_rockchip_init(struct dw_mci *host)
> @@ -95,6 +255,8 @@ static const struct dw_mci_drv_data rk3288_drv_data = {
>  	.caps			= dw_mci_rk3288_dwmmc_caps,
>  	.prepare_command        = dw_mci_rockchip_prepare_command,
>  	.set_ios		= dw_mci_rk3288_set_ios,
> +	.execute_tuning		= dw_mci_rk3288_execute_tuning,
> +	.parse_dt		= dw_mci_rk3288_parse_dt,
>  	.setup_clock    = dw_mci_rk3288_setup_clock,
>  	.init			= dw_mci_rockchip_init,
>  };
> 

Re: [PATCH v2 0/8] mmc: dw_mmc-rockchip: allow tuning using the clk-phase api

[Ulf Hansson]

On 30 September 2015 at 16:07, Heiko Stuebner <heiko@sntech.de> wrote:
> This series resurrects and adapts some individual patches whose sum
> enable the dw_mmc hosts on Rockchip socs to tune clock phases using
> the generic phase api (rk3288 and following have this capability).
>
> The changes to the original mmc-phase clocks are expanded by further
> findings resulting from devices being used in the field.
>
> Similarly the regulator handling changes do use the brand new
> regulator_set_voltage_triplet call to allow specifying lower and upper
> limits. One possible point of discussion are the two voltage ranges
> that are tried for the 3.3V signal level. Trying to stay near vmmc
> at first and only then opening the range to the full 2.7-3.6V.
>
> This mainly circumvents a shortcoming of the regulator voltage
> setting, in that even with regulator_set_voltage_triplet the regulator
> framework will take the lowest possible voltage when the possible
> voltages are below the target voltage. While it may be ideal to solve
> this on the regulator side, I'm not seeing this appearing in the short
> term, mainly because all regulator parts (including regulator drivers)
> are keyed to selecting the lowest voltage from the range, while on the
> mmc side we know which voltages may work and trying this in two steps
> does not create to much overhead, as unsupported voltages are already
> filtered out by the regulator_is_voltage_selected calls.
>
> changes since v1:
> - address comment from Jaehoon Chung and keep this local to Rockchip
>   for the time being
> - address comments from Ulf and Doug in making it more explicit
>   that the two-step 3.3V voltage setting essentially works around
>   a limitation of regulator_set_voltage_triplet
>
> Alexandru M Stan (3):
>   mmc: dw_mmc-rockchip: dt-binding: Add tuning related things
>   mmc: dw_mmc: Generic MMC tuning with the clock phase framework
>   ARM: dts: rockchip: Add drive/sample clocks for rk3288 dw_mmc devices
>
> Douglas Anderson (4):
>   clk: rockchip: Allow more precision for some mmc clock phases
>   clk: rockchip: Make calculations use rounding
>   mmc: core: Add mmc_regulator_set_vqmmc()
>   mmc: dw_mmc: Use mmc_regulator_set_vqmmc in
>     start_signal_voltage_switch
>
> Heiko Stuebner (1):
>   ARM: dts: rockchip: add tuning related settings to veyron devices
>
>  .../devicetree/bindings/mmc/rockchip-dw-mshc.txt   |  13 ++
>  arch/arm/boot/dts/rk3288-veyron-sdmmc.dtsi         |   7 +-
>  arch/arm/boot/dts/rk3288-veyron.dtsi               |   6 +
>  arch/arm/boot/dts/rk3288.dtsi                      |  20 ++-
>  drivers/clk/rockchip/clk-mmc-phase.c               |  54 ++++---
>  drivers/mmc/core/core.c                            |  74 ++++++++++
>  drivers/mmc/host/dw_mmc-rockchip.c                 | 162 +++++++++++++++++++++
>  drivers/mmc/host/dw_mmc.c                          |  17 +--
>  include/linux/mmc/host.h                           |   7 +
>  9 files changed, 321 insertions(+), 39 deletions(-)
>
> --
> 2.5.1
>

Thanks, applied for next!

Kind regards
Uffe

Re: [PATCH v2 0/8] mmc: dw_mmc-rockchip: allow tuning using the clk-phase api

[Ulf Hansson]

On 30 September 2015 at 16:07, Heiko Stuebner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org> wrote:
> This series resurrects and adapts some individual patches whose sum
> enable the dw_mmc hosts on Rockchip socs to tune clock phases using
> the generic phase api (rk3288 and following have this capability).
>
> The changes to the original mmc-phase clocks are expanded by further
> findings resulting from devices being used in the field.
>
> Similarly the regulator handling changes do use the brand new
> regulator_set_voltage_triplet call to allow specifying lower and upper
> limits. One possible point of discussion are the two voltage ranges
> that are tried for the 3.3V signal level. Trying to stay near vmmc
> at first and only then opening the range to the full 2.7-3.6V.
>
> This mainly circumvents a shortcoming of the regulator voltage
> setting, in that even with regulator_set_voltage_triplet the regulator
> framework will take the lowest possible voltage when the possible
> voltages are below the target voltage. While it may be ideal to solve
> this on the regulator side, I'm not seeing this appearing in the short
> term, mainly because all regulator parts (including regulator drivers)
> are keyed to selecting the lowest voltage from the range, while on the
> mmc side we know which voltages may work and trying this in two steps
> does not create to much overhead, as unsupported voltages are already
> filtered out by the regulator_is_voltage_selected calls.
>
> changes since v1:
> - address comment from Jaehoon Chung and keep this local to Rockchip
>   for the time being
> - address comments from Ulf and Doug in making it more explicit
>   that the two-step 3.3V voltage setting essentially works around
>   a limitation of regulator_set_voltage_triplet
>
> Alexandru M Stan (3):
>   mmc: dw_mmc-rockchip: dt-binding: Add tuning related things
>   mmc: dw_mmc: Generic MMC tuning with the clock phase framework
>   ARM: dts: rockchip: Add drive/sample clocks for rk3288 dw_mmc devices
>
> Douglas Anderson (4):
>   clk: rockchip: Allow more precision for some mmc clock phases
>   clk: rockchip: Make calculations use rounding
>   mmc: core: Add mmc_regulator_set_vqmmc()
>   mmc: dw_mmc: Use mmc_regulator_set_vqmmc in
>     start_signal_voltage_switch
>
> Heiko Stuebner (1):
>   ARM: dts: rockchip: add tuning related settings to veyron devices
>
>  .../devicetree/bindings/mmc/rockchip-dw-mshc.txt   |  13 ++
>  arch/arm/boot/dts/rk3288-veyron-sdmmc.dtsi         |   7 +-
>  arch/arm/boot/dts/rk3288-veyron.dtsi               |   6 +
>  arch/arm/boot/dts/rk3288.dtsi                      |  20 ++-
>  drivers/clk/rockchip/clk-mmc-phase.c               |  54 ++++---
>  drivers/mmc/core/core.c                            |  74 ++++++++++
>  drivers/mmc/host/dw_mmc-rockchip.c                 | 162 +++++++++++++++++++++
>  drivers/mmc/host/dw_mmc.c                          |  17 +--
>  include/linux/mmc/host.h                           |   7 +
>  9 files changed, 321 insertions(+), 39 deletions(-)
>
> --
> 2.5.1
>

Thanks, applied for next!

Kind regards
Uffe

[PATCH v2 0/8] mmc: dw_mmc-rockchip: allow tuning using the clk-phase api

[Ulf Hansson]

On 30 September 2015 at 16:07, Heiko Stuebner <heiko@sntech.de> wrote:
> This series resurrects and adapts some individual patches whose sum
> enable the dw_mmc hosts on Rockchip socs to tune clock phases using
> the generic phase api (rk3288 and following have this capability).
>
> The changes to the original mmc-phase clocks are expanded by further
> findings resulting from devices being used in the field.
>
> Similarly the regulator handling changes do use the brand new
> regulator_set_voltage_triplet call to allow specifying lower and upper
> limits. One possible point of discussion are the two voltage ranges
> that are tried for the 3.3V signal level. Trying to stay near vmmc
> at first and only then opening the range to the full 2.7-3.6V.
>
> This mainly circumvents a shortcoming of the regulator voltage
> setting, in that even with regulator_set_voltage_triplet the regulator
> framework will take the lowest possible voltage when the possible
> voltages are below the target voltage. While it may be ideal to solve
> this on the regulator side, I'm not seeing this appearing in the short
> term, mainly because all regulator parts (including regulator drivers)
> are keyed to selecting the lowest voltage from the range, while on the
> mmc side we know which voltages may work and trying this in two steps
> does not create to much overhead, as unsupported voltages are already
> filtered out by the regulator_is_voltage_selected calls.
>
> changes since v1:
> - address comment from Jaehoon Chung and keep this local to Rockchip
>   for the time being
> - address comments from Ulf and Doug in making it more explicit
>   that the two-step 3.3V voltage setting essentially works around
>   a limitation of regulator_set_voltage_triplet
>
> Alexandru M Stan (3):
>   mmc: dw_mmc-rockchip: dt-binding: Add tuning related things
>   mmc: dw_mmc: Generic MMC tuning with the clock phase framework
>   ARM: dts: rockchip: Add drive/sample clocks for rk3288 dw_mmc devices
>
> Douglas Anderson (4):
>   clk: rockchip: Allow more precision for some mmc clock phases
>   clk: rockchip: Make calculations use rounding
>   mmc: core: Add mmc_regulator_set_vqmmc()
>   mmc: dw_mmc: Use mmc_regulator_set_vqmmc in
>     start_signal_voltage_switch
>
> Heiko Stuebner (1):
>   ARM: dts: rockchip: add tuning related settings to veyron devices
>
>  .../devicetree/bindings/mmc/rockchip-dw-mshc.txt   |  13 ++
>  arch/arm/boot/dts/rk3288-veyron-sdmmc.dtsi         |   7 +-
>  arch/arm/boot/dts/rk3288-veyron.dtsi               |   6 +
>  arch/arm/boot/dts/rk3288.dtsi                      |  20 ++-
>  drivers/clk/rockchip/clk-mmc-phase.c               |  54 ++++---
>  drivers/mmc/core/core.c                            |  74 ++++++++++
>  drivers/mmc/host/dw_mmc-rockchip.c                 | 162 +++++++++++++++++++++
>  drivers/mmc/host/dw_mmc.c                          |  17 +--
>  include/linux/mmc/host.h                           |   7 +
>  9 files changed, 321 insertions(+), 39 deletions(-)
>
> --
> 2.5.1
>

Thanks, applied for next!

Kind regards
Uffe

Re: [PATCH v2 0/8] mmc: dw_mmc-rockchip: allow tuning using the clk-phase api

[Ulf Hansson]

On 8 October 2015 at 19:36, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> On 30 September 2015 at 16:07, Heiko Stuebner <heiko@sntech.de> wrote:
>> This series resurrects and adapts some individual patches whose sum
>> enable the dw_mmc hosts on Rockchip socs to tune clock phases using
>> the generic phase api (rk3288 and following have this capability).
>>
>> The changes to the original mmc-phase clocks are expanded by further
>> findings resulting from devices being used in the field.
>>
>> Similarly the regulator handling changes do use the brand new
>> regulator_set_voltage_triplet call to allow specifying lower and upper
>> limits. One possible point of discussion are the two voltage ranges
>> that are tried for the 3.3V signal level. Trying to stay near vmmc
>> at first and only then opening the range to the full 2.7-3.6V.
>>
>> This mainly circumvents a shortcoming of the regulator voltage
>> setting, in that even with regulator_set_voltage_triplet the regulator
>> framework will take the lowest possible voltage when the possible
>> voltages are below the target voltage. While it may be ideal to solve
>> this on the regulator side, I'm not seeing this appearing in the short
>> term, mainly because all regulator parts (including regulator drivers)
>> are keyed to selecting the lowest voltage from the range, while on the
>> mmc side we know which voltages may work and trying this in two steps
>> does not create to much overhead, as unsupported voltages are already
>> filtered out by the regulator_is_voltage_selected calls.
>>
>> changes since v1:
>> - address comment from Jaehoon Chung and keep this local to Rockchip
>>   for the time being
>> - address comments from Ulf and Doug in making it more explicit
>>   that the two-step 3.3V voltage setting essentially works around
>>   a limitation of regulator_set_voltage_triplet
>>
>> Alexandru M Stan (3):
>>   mmc: dw_mmc-rockchip: dt-binding: Add tuning related things
>>   mmc: dw_mmc: Generic MMC tuning with the clock phase framework
>>   ARM: dts: rockchip: Add drive/sample clocks for rk3288 dw_mmc devices
>>
>> Douglas Anderson (4):
>>   clk: rockchip: Allow more precision for some mmc clock phases
>>   clk: rockchip: Make calculations use rounding
>>   mmc: core: Add mmc_regulator_set_vqmmc()
>>   mmc: dw_mmc: Use mmc_regulator_set_vqmmc in
>>     start_signal_voltage_switch
>>
>> Heiko Stuebner (1):
>>   ARM: dts: rockchip: add tuning related settings to veyron devices
>>
>>  .../devicetree/bindings/mmc/rockchip-dw-mshc.txt   |  13 ++
>>  arch/arm/boot/dts/rk3288-veyron-sdmmc.dtsi         |   7 +-
>>  arch/arm/boot/dts/rk3288-veyron.dtsi               |   6 +
>>  arch/arm/boot/dts/rk3288.dtsi                      |  20 ++-
>>  drivers/clk/rockchip/clk-mmc-phase.c               |  54 ++++---
>>  drivers/mmc/core/core.c                            |  74 ++++++++++
>>  drivers/mmc/host/dw_mmc-rockchip.c                 | 162 +++++++++++++++++++++
>>  drivers/mmc/host/dw_mmc.c                          |  17 +--
>>  include/linux/mmc/host.h                           |   7 +
>>  9 files changed, 321 insertions(+), 39 deletions(-)
>>
>> --
>> 2.5.1
>>
>
> Thanks, applied for next!
>
> Kind regards
> Uffe

Dough, Heiko,

This patchset seems to be causing a boot regression for exynos5800-peach-pi [1].

Apparently, the vmmc regulator doesn't exist for one of the controller
but vqmmc is. This leads to the following NULL pointer exception.

.....

[    5.672281] dwmmc_exynos 12210000.mmc: No vmmc regulator found
[    5.678234] dwmmc_exynos 12210000.mmc: allocated mmc-pwrseq
[    5.683890] Unable to handle kernel NULL pointer dereference at
virtual address 0000001d
[    5.691727] pgd = c0204000
[    5.694296] [0000001d] *pgd=00000000
[    5.697853] Internal error: Oops: 17 [#1] SMP ARM
[    5.702538] Modules linked in: snd_soc_max98090 exynosdrm
snd_soc_core rtc_max77802 max77802 spi_s3c64xx snd_compress
snd_pcm_dmaengine snd_pcm pwm_samsung snd_timer clk_max77802 snd
soundcore rtc_s3c exynos_adc phy_exynos_usb2 ohci_exynos
[    5.723702] CPU: 6 PID: 83 Comm: kworker/u16:1 Not tainted
4.3.0-rc4-00051-gaf00b2580295 #1
[    5.732053] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
[    5.738120] Workqueue: deferwq deferred_probe_work_func
[    5.743305] task: ed7297c0 ti: ed780000 task.ti: ed780000
[    5.746177] ntc-thermistor 12d10000.adc:ncp15wb473@3: Thermistor
type: ncp15wb473 successfully probed.
[    5.746514] ntc-thermistor 12d10000.adc:ncp15wb473@4: Thermistor
type: ncp15wb473 successfully probed.
[    5.746913] ntc-thermistor 12d10000.adc:ncp15wb473@5: Thermistor
type: ncp15wb473 successfully probed.
[    5.747150] ntc-thermistor 12d10000.adc:ncp15wb473@6: Thermistor
type: ncp15wb473 successfully probed.
[    5.785853] PC is at regulator_get_voltage+0x8/0x34
[    5.790681] LR is at mmc_regulator_set_vqmmc+0x40/0xe8
[    5.795798] pc : [<c0553bcc>]    lr : [<c080b820>]    psr: 80000013
[    5.795798] sp : ed781dd0  ip : ec02e5f8  fp : 00000000
[    5.807269] r10: 00000000  r9 : ec02e700  r8 : 00000000
[    5.812450] r7 : 00000000  r6 : 00000001  r5 : 00000000  r4 : ffffffed
[    5.818958] r3 : ec02e400  r2 : 00000000  r1 : ec02e5f8  r0 : ffffffed
[    5.825466] Flags: Nzcv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment none
[    5.832579] Control: 10c5387d  Table: 6c18006a  DAC: 00000051
[    5.838303] Process kworker/u16:1 (pid: 83, stack limit = 0xed780220)
[    5.844722] Stack: (0xed781dd0 to 0xed782000)
[    5.849061] 1dc0:                                     ec02e400
ec02e400 00000000 c080b820
[    5.857240] 1de0: ec57da10 00000000 00000001 00000000 00000000
c08330e0 c0833074 ec02e400
[    5.865395] 1e00: ec02e5f8 c080cb58 00000000 ec02e400 00000000
ec02e400 00000000 c080d83c
[    5.873550] 1e20: ec02e400 c080e8b8 00000001 ec57da10 00000000
c083460c 00000040 c0cbe7e0
[    5.881704] 1e40: ec57da10 ed223bc0 ed23d410 00000000 c0ac26a8
17d78400 edfc15b4 ec57da10
[    5.889858] 1e60: ec57da10 fffffffe ed23d410 fffffdfb c0f3a920
0000000b ed723080 00000001
[    5.898013] 1e80: 00000001 c0623668 c0623620 ed23d410 c0fbde24
00000000 c0f3a920 c0622098
[    5.906168] 1ea0: 00000000 ed781ed0 c0622344 00000001 00000000
c0620730 ed08b770 ed3836b8
[    5.914323] 1ec0: ed23d410 ed23d444 c0f1a910 c0621e2c ed23d410
00000001 00000002 ed23d410
[    5.922477] 1ee0: ed23d410 c0f1a910 ed7a3800 c06215a8 ed23d410
c0f1a8b0 c0f1a898 c0621980
[    5.930631] 1f00: ed723080 c0f1a8cc ed01b800 c025ed00 ed781f5c
ed01b800 00000001 ed01b814
[    5.938785] 1f20: 00000001 ed01b800 ed723098 00000088 ed723080
ed01b800 00000001 c025ef60
[    5.946940] 1f40: ed7297c0 ed762000 00000000 ed723080 c025ef38
00000000 00000000 00000000
[    5.955094] 1f60: 00000000 c02640a0 ed083dc0 00000000 00000000
ed723080 00000000 00000000
[    5.963249] 1f80: ed781f80 ed781f80 00000000 00000000 ed781f90
ed781f90 ed781fac ed762000
[    5.971403] 1fa0: c0263fc8 00000000 00000000 c0210b38 00000000
00000000 00000000 00000000
[    5.979557] 1fc0: 00000000 00000000 00000000 00000000 00000000
00000000 00000000 00000000
[    5.987712] 1fe0: 00000000 00000000 00000000 00000000 00000013
00000000 00000000 00000000
[    5.995883] [<c0553bcc>] (regulator_get_voltage) from [<c080b820>]
(mmc_regulator_set_vqmmc+0x40/0xe8)
[    6.005158] [<c080b820>] (mmc_regulator_set_vqmmc) from
[<c08330e0>] (dw_mci_switch_voltage+0x6c/0x84)
[    6.014438] [<c08330e0>] (dw_mci_switch_voltage) from [<c080cb58>]
(mmc_power_up+0x68/0x104)
[    6.022852] [<c080cb58>] (mmc_power_up) from [<c080d83c>]
(mmc_start_host+0x50/0x80)
[    6.030569] [<c080d83c>] (mmc_start_host) from [<c080e8b8>]
(mmc_add_host+0x58/0x7c)
[    6.038289] [<c080e8b8>] (mmc_add_host) from [<c083460c>]
(dw_mci_probe+0x5d8/0xc68)
[    6.046013] [<c083460c>] (dw_mci_probe) from [<c0623668>]
(platform_drv_probe+0x48/0xa4)
[    6.054082] [<c0623668>] (platform_drv_probe) from [<c0622098>]
(driver_probe_device+0x1e0/0x2a0)
[    6.062930] [<c0622098>] (driver_probe_device) from [<c0620730>]
(bus_for_each_drv+0x44/0x8c)
[    6.071434] [<c0620730>] (bus_for_each_drv) from [<c0621e2c>]
(__device_attach+0xa0/0x104)
[    6.079670] [<c0621e2c>] (__device_attach) from [<c06215a8>]
(bus_probe_device+0x84/0x8c)
[    6.087825] [<c06215a8>] (bus_probe_device) from [<c0621980>]
(deferred_probe_work_func+0x5c/0x88)
[    6.096763] [<c0621980>] (deferred_probe_work_func) from
[<c025ed00>] (process_one_work+0x134/0x338)
[    6.105871] [<c025ed00>] (process_one_work) from [<c025ef60>]
(worker_thread+0x28/0x4cc)
[    6.113938] [<c025ef60>] (worker_thread) from [<c02640a0>]
(kthread+0xd8/0xf4)
[    6.121139] [<c02640a0>] (kthread) from [<c0210b38>]
(ret_from_fork+0x14/0x3c)
[    6.128333] Code: e5843010 eaffffdb e92d4038 e1a04000 (e5900030)
[    6.134460] ---[ end trace ee25ca3d5cf9ee44 ]---

....

Looking into the new mmc_regulator_set_vqmmc() API, I believe it's
quite obvious where the error is, as there's a missing validation of
"mmc->supply.vmmc" before trying regulator_get_voltage() for it.

I think this is the proper solution:
- Instead of using the "mmc->supply.vmmc" to fetch the current voltage
level, let's use "mmc->ios.vdd" (we need to translate the bit into a
voltage levels), which thus make the new the API independent of valid
regulator for vmmc.

Until we fixed that, I am dropping patch3 and onwards.

Kind regards
Uffe

[1]
http://kernelci.org/boot/exynos5800-peach-pi/job/ulfh/kernel/v4.3-rc4-51-gaf00b2580295/defconfig/multi_v7_defconfig/

[PATCH v2 0/8] mmc: dw_mmc-rockchip: allow tuning using the clk-phase api

[Ulf Hansson]

On 8 October 2015 at 19:36, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> On 30 September 2015 at 16:07, Heiko Stuebner <heiko@sntech.de> wrote:
>> This series resurrects and adapts some individual patches whose sum
>> enable the dw_mmc hosts on Rockchip socs to tune clock phases using
>> the generic phase api (rk3288 and following have this capability).
>>
>> The changes to the original mmc-phase clocks are expanded by further
>> findings resulting from devices being used in the field.
>>
>> Similarly the regulator handling changes do use the brand new
>> regulator_set_voltage_triplet call to allow specifying lower and upper
>> limits. One possible point of discussion are the two voltage ranges
>> that are tried for the 3.3V signal level. Trying to stay near vmmc
>> at first and only then opening the range to the full 2.7-3.6V.
>>
>> This mainly circumvents a shortcoming of the regulator voltage
>> setting, in that even with regulator_set_voltage_triplet the regulator
>> framework will take the lowest possible voltage when the possible
>> voltages are below the target voltage. While it may be ideal to solve
>> this on the regulator side, I'm not seeing this appearing in the short
>> term, mainly because all regulator parts (including regulator drivers)
>> are keyed to selecting the lowest voltage from the range, while on the
>> mmc side we know which voltages may work and trying this in two steps
>> does not create to much overhead, as unsupported voltages are already
>> filtered out by the regulator_is_voltage_selected calls.
>>
>> changes since v1:
>> - address comment from Jaehoon Chung and keep this local to Rockchip
>>   for the time being
>> - address comments from Ulf and Doug in making it more explicit
>>   that the two-step 3.3V voltage setting essentially works around
>>   a limitation of regulator_set_voltage_triplet
>>
>> Alexandru M Stan (3):
>>   mmc: dw_mmc-rockchip: dt-binding: Add tuning related things
>>   mmc: dw_mmc: Generic MMC tuning with the clock phase framework
>>   ARM: dts: rockchip: Add drive/sample clocks for rk3288 dw_mmc devices
>>
>> Douglas Anderson (4):
>>   clk: rockchip: Allow more precision for some mmc clock phases
>>   clk: rockchip: Make calculations use rounding
>>   mmc: core: Add mmc_regulator_set_vqmmc()
>>   mmc: dw_mmc: Use mmc_regulator_set_vqmmc in
>>     start_signal_voltage_switch
>>
>> Heiko Stuebner (1):
>>   ARM: dts: rockchip: add tuning related settings to veyron devices
>>
>>  .../devicetree/bindings/mmc/rockchip-dw-mshc.txt   |  13 ++
>>  arch/arm/boot/dts/rk3288-veyron-sdmmc.dtsi         |   7 +-
>>  arch/arm/boot/dts/rk3288-veyron.dtsi               |   6 +
>>  arch/arm/boot/dts/rk3288.dtsi                      |  20 ++-
>>  drivers/clk/rockchip/clk-mmc-phase.c               |  54 ++++---
>>  drivers/mmc/core/core.c                            |  74 ++++++++++
>>  drivers/mmc/host/dw_mmc-rockchip.c                 | 162 +++++++++++++++++++++
>>  drivers/mmc/host/dw_mmc.c                          |  17 +--
>>  include/linux/mmc/host.h                           |   7 +
>>  9 files changed, 321 insertions(+), 39 deletions(-)
>>
>> --
>> 2.5.1
>>
>
> Thanks, applied for next!
>
> Kind regards
> Uffe

Dough, Heiko,

This patchset seems to be causing a boot regression for exynos5800-peach-pi [1].

Apparently, the vmmc regulator doesn't exist for one of the controller
but vqmmc is. This leads to the following NULL pointer exception.

.....

[    5.672281] dwmmc_exynos 12210000.mmc: No vmmc regulator found
[    5.678234] dwmmc_exynos 12210000.mmc: allocated mmc-pwrseq
[    5.683890] Unable to handle kernel NULL pointer dereference at
virtual address 0000001d
[    5.691727] pgd = c0204000
[    5.694296] [0000001d] *pgd=00000000
[    5.697853] Internal error: Oops: 17 [#1] SMP ARM
[    5.702538] Modules linked in: snd_soc_max98090 exynosdrm
snd_soc_core rtc_max77802 max77802 spi_s3c64xx snd_compress
snd_pcm_dmaengine snd_pcm pwm_samsung snd_timer clk_max77802 snd
soundcore rtc_s3c exynos_adc phy_exynos_usb2 ohci_exynos
[    5.723702] CPU: 6 PID: 83 Comm: kworker/u16:1 Not tainted
4.3.0-rc4-00051-gaf00b2580295 #1
[    5.732053] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
[    5.738120] Workqueue: deferwq deferred_probe_work_func
[    5.743305] task: ed7297c0 ti: ed780000 task.ti: ed780000
[    5.746177] ntc-thermistor 12d10000.adc:ncp15wb473 at 3: Thermistor
type: ncp15wb473 successfully probed.
[    5.746514] ntc-thermistor 12d10000.adc:ncp15wb473 at 4: Thermistor
type: ncp15wb473 successfully probed.
[    5.746913] ntc-thermistor 12d10000.adc:ncp15wb473 at 5: Thermistor
type: ncp15wb473 successfully probed.
[    5.747150] ntc-thermistor 12d10000.adc:ncp15wb473 at 6: Thermistor
type: ncp15wb473 successfully probed.
[    5.785853] PC is at regulator_get_voltage+0x8/0x34
[    5.790681] LR is at mmc_regulator_set_vqmmc+0x40/0xe8
[    5.795798] pc : [<c0553bcc>]    lr : [<c080b820>]    psr: 80000013
[    5.795798] sp : ed781dd0  ip : ec02e5f8  fp : 00000000
[    5.807269] r10: 00000000  r9 : ec02e700  r8 : 00000000
[    5.812450] r7 : 00000000  r6 : 00000001  r5 : 00000000  r4 : ffffffed
[    5.818958] r3 : ec02e400  r2 : 00000000  r1 : ec02e5f8  r0 : ffffffed
[    5.825466] Flags: Nzcv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment none
[    5.832579] Control: 10c5387d  Table: 6c18006a  DAC: 00000051
[    5.838303] Process kworker/u16:1 (pid: 83, stack limit = 0xed780220)
[    5.844722] Stack: (0xed781dd0 to 0xed782000)
[    5.849061] 1dc0:                                     ec02e400
ec02e400 00000000 c080b820
[    5.857240] 1de0: ec57da10 00000000 00000001 00000000 00000000
c08330e0 c0833074 ec02e400
[    5.865395] 1e00: ec02e5f8 c080cb58 00000000 ec02e400 00000000
ec02e400 00000000 c080d83c
[    5.873550] 1e20: ec02e400 c080e8b8 00000001 ec57da10 00000000
c083460c 00000040 c0cbe7e0
[    5.881704] 1e40: ec57da10 ed223bc0 ed23d410 00000000 c0ac26a8
17d78400 edfc15b4 ec57da10
[    5.889858] 1e60: ec57da10 fffffffe ed23d410 fffffdfb c0f3a920
0000000b ed723080 00000001
[    5.898013] 1e80: 00000001 c0623668 c0623620 ed23d410 c0fbde24
00000000 c0f3a920 c0622098
[    5.906168] 1ea0: 00000000 ed781ed0 c0622344 00000001 00000000
c0620730 ed08b770 ed3836b8
[    5.914323] 1ec0: ed23d410 ed23d444 c0f1a910 c0621e2c ed23d410
00000001 00000002 ed23d410
[    5.922477] 1ee0: ed23d410 c0f1a910 ed7a3800 c06215a8 ed23d410
c0f1a8b0 c0f1a898 c0621980
[    5.930631] 1f00: ed723080 c0f1a8cc ed01b800 c025ed00 ed781f5c
ed01b800 00000001 ed01b814
[    5.938785] 1f20: 00000001 ed01b800 ed723098 00000088 ed723080
ed01b800 00000001 c025ef60
[    5.946940] 1f40: ed7297c0 ed762000 00000000 ed723080 c025ef38
00000000 00000000 00000000
[    5.955094] 1f60: 00000000 c02640a0 ed083dc0 00000000 00000000
ed723080 00000000 00000000
[    5.963249] 1f80: ed781f80 ed781f80 00000000 00000000 ed781f90
ed781f90 ed781fac ed762000
[    5.971403] 1fa0: c0263fc8 00000000 00000000 c0210b38 00000000
00000000 00000000 00000000
[    5.979557] 1fc0: 00000000 00000000 00000000 00000000 00000000
00000000 00000000 00000000
[    5.987712] 1fe0: 00000000 00000000 00000000 00000000 00000013
00000000 00000000 00000000
[    5.995883] [<c0553bcc>] (regulator_get_voltage) from [<c080b820>]
(mmc_regulator_set_vqmmc+0x40/0xe8)
[    6.005158] [<c080b820>] (mmc_regulator_set_vqmmc) from
[<c08330e0>] (dw_mci_switch_voltage+0x6c/0x84)
[    6.014438] [<c08330e0>] (dw_mci_switch_voltage) from [<c080cb58>]
(mmc_power_up+0x68/0x104)
[    6.022852] [<c080cb58>] (mmc_power_up) from [<c080d83c>]
(mmc_start_host+0x50/0x80)
[    6.030569] [<c080d83c>] (mmc_start_host) from [<c080e8b8>]
(mmc_add_host+0x58/0x7c)
[    6.038289] [<c080e8b8>] (mmc_add_host) from [<c083460c>]
(dw_mci_probe+0x5d8/0xc68)
[    6.046013] [<c083460c>] (dw_mci_probe) from [<c0623668>]
(platform_drv_probe+0x48/0xa4)
[    6.054082] [<c0623668>] (platform_drv_probe) from [<c0622098>]
(driver_probe_device+0x1e0/0x2a0)
[    6.062930] [<c0622098>] (driver_probe_device) from [<c0620730>]
(bus_for_each_drv+0x44/0x8c)
[    6.071434] [<c0620730>] (bus_for_each_drv) from [<c0621e2c>]
(__device_attach+0xa0/0x104)
[    6.079670] [<c0621e2c>] (__device_attach) from [<c06215a8>]
(bus_probe_device+0x84/0x8c)
[    6.087825] [<c06215a8>] (bus_probe_device) from [<c0621980>]
(deferred_probe_work_func+0x5c/0x88)
[    6.096763] [<c0621980>] (deferred_probe_work_func) from
[<c025ed00>] (process_one_work+0x134/0x338)
[    6.105871] [<c025ed00>] (process_one_work) from [<c025ef60>]
(worker_thread+0x28/0x4cc)
[    6.113938] [<c025ef60>] (worker_thread) from [<c02640a0>]
(kthread+0xd8/0xf4)
[    6.121139] [<c02640a0>] (kthread) from [<c0210b38>]
(ret_from_fork+0x14/0x3c)
[    6.128333] Code: e5843010 eaffffdb e92d4038 e1a04000 (e5900030)
[    6.134460] ---[ end trace ee25ca3d5cf9ee44 ]---

....

Looking into the new mmc_regulator_set_vqmmc() API, I believe it's
quite obvious where the error is, as there's a missing validation of
"mmc->supply.vmmc" before trying regulator_get_voltage() for it.

I think this is the proper solution:
- Instead of using the "mmc->supply.vmmc" to fetch the current voltage
level, let's use "mmc->ios.vdd" (we need to translate the bit into a
voltage levels), which thus make the new the API independent of valid
regulator for vmmc.

Until we fixed that, I am dropping patch3 and onwards.

Kind regards
Uffe

[1]
http://kernelci.org/boot/exynos5800-peach-pi/job/ulfh/kernel/v4.3-rc4-51-gaf00b2580295/defconfig/multi_v7_defconfig/

Re: [PATCH v2 0/8] mmc: dw_mmc-rockchip: allow tuning using the clk-phase api

[Doug Anderson]

Hi,

On Fri, Oct 9, 2015 at 1:10 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> Dough, Heiko,
>
> This patchset seems to be causing a boot regression for exynos5800-peach-pi [1].
>
> Apparently, the vmmc regulator doesn't exist for one of the controller
> but vqmmc is. This leads to the following NULL pointer exception.

Ugh.  Hrm, it didn't used to be possible for vmmc to be NULL.  :-/  It
changed in v3.16:

  4d1f52f mmc: core: Improve support for deferred regulators

...and I didn't notice it (most of my development is done on 3.14).  :(


Is it sensible to have vqmmc without vmmc.  One option would be to
change mmc_regulator_get_supply() to not even bother trying to get
vqmmc if vmmc is not there.

Another option would be to just totally fail mmc_regulator_set_vqmmc()
always if vmmc is an error.  ...or assume that any bogus vmmc (either
vmmc is an error or regulator_get_voltage(vmmc) returns an error) is
3.3V?


I guess the safest is to assume that bogus vmmc means 3.3V...


-Doug

[PATCH v2 0/8] mmc: dw_mmc-rockchip: allow tuning using the clk-phase api

[Doug Anderson]

Hi,

On Fri, Oct 9, 2015 at 1:10 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> Dough, Heiko,
>
> This patchset seems to be causing a boot regression for exynos5800-peach-pi [1].
>
> Apparently, the vmmc regulator doesn't exist for one of the controller
> but vqmmc is. This leads to the following NULL pointer exception.

Ugh.  Hrm, it didn't used to be possible for vmmc to be NULL.  :-/  It
changed in v3.16:

  4d1f52f mmc: core: Improve support for deferred regulators

...and I didn't notice it (most of my development is done on 3.14).  :(


Is it sensible to have vqmmc without vmmc.  One option would be to
change mmc_regulator_get_supply() to not even bother trying to get
vqmmc if vmmc is not there.

Another option would be to just totally fail mmc_regulator_set_vqmmc()
always if vmmc is an error.  ...or assume that any bogus vmmc (either
vmmc is an error or regulator_get_voltage(vmmc) returns an error) is
3.3V?


I guess the safest is to assume that bogus vmmc means 3.3V...


-Doug

Re: [PATCH v2 4/8] mmc: dw_mmc: Use mmc_regulator_set_vqmmc in start_signal_voltage_switch

[Kevin Hilman]

On Wed, Sep 30, 2015 at 4:07 PM, Heiko Stuebner <heiko@sntech.de> wrote:
> From: Douglas Anderson <dianders@chromium.org>
>
> We've introduced a new helper in the MMC core:
> mmc_regulator_set_vqmmc().  Let's use this in dw_mmc.  Using this new
> helper has some advantages:
>
> 1. We get the mmc_regulator_set_vqmmc() behavior of trying to match
>    VQMMC and VMMC when the signal voltage is 3.3V.  This ensures max
>    compatibility.
>
> 2. We get rid of a few more warnings when probing unsupported
>    voltages.
>
> 3. We get rid of some non-dw_mmc specific code in dw_mmc.
>
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>

kernelci.org found a new failure in linux-next on
exynos5800-peach-pi[1] and it was bisected down to this patch, which
has hit linux-next in the form of commit fba2e23128d0 (mmc: dw_mmc:
Use mmc_regulator_set_vqmmc in start_signal_voltage_switch).  Based on
a quick glance at the boot log[2], it looks to be a bad pointer
dereference in regulator_get_voltage().

I confirmed that reverting this patch on top of next-20151009 gets my
peach-pi booting again.

Note that the boot failure is only on exynos_defconfig, not multi_v7_defconfig.

Kevin


[1] http://kernelci.org/boot/exynos5800-peach-pi/job/next/kernel/next-20151009/defconfig/exynos_defconfig/lab/lab-khilman/?_id=56177d3659b514a50c6c3267

[2] http://storage.kernelci.org/next/next-20151009/arm-exynos_defconfig/lab-khilman/boot-exynos5800-peach-pi.html


> ---
>  drivers/mmc/host/dw_mmc.c | 17 ++++++-----------
>  1 file changed, 6 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index fcbf552..b1b7e7f 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -1279,7 +1279,6 @@ static int dw_mci_switch_voltage(struct mmc_host *mmc, struct mmc_ios *ios)
>         const struct dw_mci_drv_data *drv_data = host->drv_data;
>         u32 uhs;
>         u32 v18 = SDMMC_UHS_18V << slot->id;
> -       int min_uv, max_uv;
>         int ret;
>
>         if (drv_data && drv_data->switch_voltage)
> @@ -1291,22 +1290,18 @@ static int dw_mci_switch_voltage(struct mmc_host *mmc, struct mmc_ios *ios)
>          * does no harm but you need to set the regulator directly.  Try both.
>          */
>         uhs = mci_readl(host, UHS_REG);
> -       if (ios->signal_voltage == MMC_SIGNAL_VOLTAGE_330) {
> -               min_uv = 2700000;
> -               max_uv = 3600000;
> +       if (ios->signal_voltage == MMC_SIGNAL_VOLTAGE_330)
>                 uhs &= ~v18;
> -       } else {
> -               min_uv = 1700000;
> -               max_uv = 1950000;
> +       else
>                 uhs |= v18;
> -       }
> +
>         if (!IS_ERR(mmc->supply.vqmmc)) {
> -               ret = regulator_set_voltage(mmc->supply.vqmmc, min_uv, max_uv);
> +               ret = mmc_regulator_set_vqmmc(mmc, ios);
>
>                 if (ret) {
>                         dev_dbg(&mmc->class_dev,
> -                                        "Regulator set error %d: %d - %d\n",
> -                                        ret, min_uv, max_uv);
> +                                        "Regulator set error %d - %s V\n",
> +                                        ret, uhs & v18 ? "1.8" : "3.3");
>                         return ret;
>                 }
>         }
> --
> 2.5.1
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 4/8] mmc: dw_mmc: Use mmc_regulator_set_vqmmc in start_signal_voltage_switch

[Kevin Hilman]

On Wed, Sep 30, 2015 at 4:07 PM, Heiko Stuebner <heiko@sntech.de> wrote:
> From: Douglas Anderson <dianders@chromium.org>
>
> We've introduced a new helper in the MMC core:
> mmc_regulator_set_vqmmc().  Let's use this in dw_mmc.  Using this new
> helper has some advantages:
>
> 1. We get the mmc_regulator_set_vqmmc() behavior of trying to match
>    VQMMC and VMMC when the signal voltage is 3.3V.  This ensures max
>    compatibility.
>
> 2. We get rid of a few more warnings when probing unsupported
>    voltages.
>
> 3. We get rid of some non-dw_mmc specific code in dw_mmc.
>
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>

kernelci.org found a new failure in linux-next on
exynos5800-peach-pi[1] and it was bisected down to this patch, which
has hit linux-next in the form of commit fba2e23128d0 (mmc: dw_mmc:
Use mmc_regulator_set_vqmmc in start_signal_voltage_switch).  Based on
a quick glance at the boot log[2], it looks to be a bad pointer
dereference in regulator_get_voltage().

I confirmed that reverting this patch on top of next-20151009 gets my
peach-pi booting again.

Note that the boot failure is only on exynos_defconfig, not multi_v7_defconfig.

Kevin


[1] http://kernelci.org/boot/exynos5800-peach-pi/job/next/kernel/next-20151009/defconfig/exynos_defconfig/lab/lab-khilman/?_id=56177d3659b514a50c6c3267

[2] http://storage.kernelci.org/next/next-20151009/arm-exynos_defconfig/lab-khilman/boot-exynos5800-peach-pi.html


> ---
>  drivers/mmc/host/dw_mmc.c | 17 ++++++-----------
>  1 file changed, 6 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index fcbf552..b1b7e7f 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -1279,7 +1279,6 @@ static int dw_mci_switch_voltage(struct mmc_host *mmc, struct mmc_ios *ios)
>         const struct dw_mci_drv_data *drv_data = host->drv_data;
>         u32 uhs;
>         u32 v18 = SDMMC_UHS_18V << slot->id;
> -       int min_uv, max_uv;
>         int ret;
>
>         if (drv_data && drv_data->switch_voltage)
> @@ -1291,22 +1290,18 @@ static int dw_mci_switch_voltage(struct mmc_host *mmc, struct mmc_ios *ios)
>          * does no harm but you need to set the regulator directly.  Try both.
>          */
>         uhs = mci_readl(host, UHS_REG);
> -       if (ios->signal_voltage == MMC_SIGNAL_VOLTAGE_330) {
> -               min_uv = 2700000;
> -               max_uv = 3600000;
> +       if (ios->signal_voltage == MMC_SIGNAL_VOLTAGE_330)
>                 uhs &= ~v18;
> -       } else {
> -               min_uv = 1700000;
> -               max_uv = 1950000;
> +       else
>                 uhs |= v18;
> -       }
> +
>         if (!IS_ERR(mmc->supply.vqmmc)) {
> -               ret = regulator_set_voltage(mmc->supply.vqmmc, min_uv, max_uv);
> +               ret = mmc_regulator_set_vqmmc(mmc, ios);
>
>                 if (ret) {
>                         dev_dbg(&mmc->class_dev,
> -                                        "Regulator set error %d: %d - %d\n",
> -                                        ret, min_uv, max_uv);
> +                                        "Regulator set error %d - %s V\n",
> +                                        ret, uhs & v18 ? "1.8" : "3.3");
>                         return ret;
>                 }
>         }
> --
> 2.5.1
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 0/8] mmc: dw_mmc-rockchip: allow tuning using the clk-phase api

[Doug Anderson]

Hi,

On Fri, Oct 9, 2015 at 10:19 AM, Doug Anderson <dianders@chromium.org> wrote:
> Hi,
>
> On Fri, Oct 9, 2015 at 1:10 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>> Dough, Heiko,
>>
>> This patchset seems to be causing a boot regression for exynos5800-peach-pi [1].
>>
>> Apparently, the vmmc regulator doesn't exist for one of the controller
>> but vqmmc is. This leads to the following NULL pointer exception.
>
> Ugh.  Hrm, it didn't used to be possible for vmmc to be NULL.  :-/  It
> changed in v3.16:
>
>   4d1f52f mmc: core: Improve support for deferred regulators
>
> ...and I didn't notice it (most of my development is done on 3.14).  :(
>
>
> Is it sensible to have vqmmc without vmmc.  One option would be to
> change mmc_regulator_get_supply() to not even bother trying to get
> vqmmc if vmmc is not there.
>
> Another option would be to just totally fail mmc_regulator_set_vqmmc()
> always if vmmc is an error.  ...or assume that any bogus vmmc (either
> vmmc is an error or regulator_get_voltage(vmmc) returns an error) is
> 3.3V?
>
>
> I guess the safest is to assume that bogus vmmc means 3.3V...

Ah, doh.  Just noticed Ulf's suggestion all the way at the bottom of
the crash info, which I somehow missed before.  Yes, let's do that.
Heiko: are you going to re-post?

-Doug

[PATCH v2 0/8] mmc: dw_mmc-rockchip: allow tuning using the clk-phase api

[Doug Anderson]

Hi,

On Fri, Oct 9, 2015 at 10:19 AM, Doug Anderson <dianders@chromium.org> wrote:
> Hi,
>
> On Fri, Oct 9, 2015 at 1:10 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>> Dough, Heiko,
>>
>> This patchset seems to be causing a boot regression for exynos5800-peach-pi [1].
>>
>> Apparently, the vmmc regulator doesn't exist for one of the controller
>> but vqmmc is. This leads to the following NULL pointer exception.
>
> Ugh.  Hrm, it didn't used to be possible for vmmc to be NULL.  :-/  It
> changed in v3.16:
>
>   4d1f52f mmc: core: Improve support for deferred regulators
>
> ...and I didn't notice it (most of my development is done on 3.14).  :(
>
>
> Is it sensible to have vqmmc without vmmc.  One option would be to
> change mmc_regulator_get_supply() to not even bother trying to get
> vqmmc if vmmc is not there.
>
> Another option would be to just totally fail mmc_regulator_set_vqmmc()
> always if vmmc is an error.  ...or assume that any bogus vmmc (either
> vmmc is an error or regulator_get_voltage(vmmc) returns an error) is
> 3.3V?
>
>
> I guess the safest is to assume that bogus vmmc means 3.3V...

Ah, doh.  Just noticed Ulf's suggestion all the way at the bottom of
the crash info, which I somehow missed before.  Yes, let's do that.
Heiko: are you going to re-post?

-Doug

Re: [PATCH v2 4/8] mmc: dw_mmc: Use mmc_regulator_set_vqmmc in start_signal_voltage_switch

[Doug Anderson]

Kevin,

On Fri, Oct 9, 2015 at 12:06 PM, Kevin Hilman <khilman@kernel.org> wrote:
> On Wed, Sep 30, 2015 at 4:07 PM, Heiko Stuebner <heiko@sntech.de> wrote:
>> From: Douglas Anderson <dianders@chromium.org>
>>
>> We've introduced a new helper in the MMC core:
>> mmc_regulator_set_vqmmc().  Let's use this in dw_mmc.  Using this new
>> helper has some advantages:
>>
>> 1. We get the mmc_regulator_set_vqmmc() behavior of trying to match
>>    VQMMC and VMMC when the signal voltage is 3.3V.  This ensures max
>>    compatibility.
>>
>> 2. We get rid of a few more warnings when probing unsupported
>>    voltages.
>>
>> 3. We get rid of some non-dw_mmc specific code in dw_mmc.
>>
>> Signed-off-by: Douglas Anderson <dianders@chromium.org>
>> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
>
> kernelci.org found a new failure in linux-next on
> exynos5800-peach-pi[1] and it was bisected down to this patch, which
> has hit linux-next in the form of commit fba2e23128d0 (mmc: dw_mmc:
> Use mmc_regulator_set_vqmmc in start_signal_voltage_switch).  Based on
> a quick glance at the boot log[2], it looks to be a bad pointer
> dereference in regulator_get_voltage().
>
> I confirmed that reverting this patch on top of next-20151009 gets my
> peach-pi booting again.
>
> Note that the boot failure is only on exynos_defconfig, not multi_v7_defconfig.

Right.  See <http://www.spinics.net/lists/arm-kernel/msg450802.html>
for discussion.  Ulf responded to the cover letter rather than the
individual patch.

-Doug

[PATCH v2 4/8] mmc: dw_mmc: Use mmc_regulator_set_vqmmc in start_signal_voltage_switch

[Doug Anderson]

Kevin,

On Fri, Oct 9, 2015 at 12:06 PM, Kevin Hilman <khilman@kernel.org> wrote:
> On Wed, Sep 30, 2015 at 4:07 PM, Heiko Stuebner <heiko@sntech.de> wrote:
>> From: Douglas Anderson <dianders@chromium.org>
>>
>> We've introduced a new helper in the MMC core:
>> mmc_regulator_set_vqmmc().  Let's use this in dw_mmc.  Using this new
>> helper has some advantages:
>>
>> 1. We get the mmc_regulator_set_vqmmc() behavior of trying to match
>>    VQMMC and VMMC when the signal voltage is 3.3V.  This ensures max
>>    compatibility.
>>
>> 2. We get rid of a few more warnings when probing unsupported
>>    voltages.
>>
>> 3. We get rid of some non-dw_mmc specific code in dw_mmc.
>>
>> Signed-off-by: Douglas Anderson <dianders@chromium.org>
>> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
>
> kernelci.org found a new failure in linux-next on
> exynos5800-peach-pi[1] and it was bisected down to this patch, which
> has hit linux-next in the form of commit fba2e23128d0 (mmc: dw_mmc:
> Use mmc_regulator_set_vqmmc in start_signal_voltage_switch).  Based on
> a quick glance at the boot log[2], it looks to be a bad pointer
> dereference in regulator_get_voltage().
>
> I confirmed that reverting this patch on top of next-20151009 gets my
> peach-pi booting again.
>
> Note that the boot failure is only on exynos_defconfig, not multi_v7_defconfig.

Right.  See <http://www.spinics.net/lists/arm-kernel/msg450802.html>
for discussion.  Ulf responded to the cover letter rather than the
individual patch.

-Doug

Re: [PATCH v2 4/8] mmc: dw_mmc: Use mmc_regulator_set_vqmmc in start_signal_voltage_switch

[Kevin Hilman]

On Fri, Oct 9, 2015 at 10:12 PM, Doug Anderson <dianders@chromium.org> wrote:
> Kevin,
>
> On Fri, Oct 9, 2015 at 12:06 PM, Kevin Hilman <khilman@kernel.org> wrote:
>> On Wed, Sep 30, 2015 at 4:07 PM, Heiko Stuebner <heiko@sntech.de> wrote:
>>> From: Douglas Anderson <dianders@chromium.org>
>>>
>>> We've introduced a new helper in the MMC core:
>>> mmc_regulator_set_vqmmc().  Let's use this in dw_mmc.  Using this new
>>> helper has some advantages:
>>>
>>> 1. We get the mmc_regulator_set_vqmmc() behavior of trying to match
>>>    VQMMC and VMMC when the signal voltage is 3.3V.  This ensures max
>>>    compatibility.
>>>
>>> 2. We get rid of a few more warnings when probing unsupported
>>>    voltages.
>>>
>>> 3. We get rid of some non-dw_mmc specific code in dw_mmc.
>>>
>>> Signed-off-by: Douglas Anderson <dianders@chromium.org>
>>> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
>>
>> kernelci.org found a new failure in linux-next on
>> exynos5800-peach-pi[1] and it was bisected down to this patch, which
>> has hit linux-next in the form of commit fba2e23128d0 (mmc: dw_mmc:
>> Use mmc_regulator_set_vqmmc in start_signal_voltage_switch).  Based on
>> a quick glance at the boot log[2], it looks to be a bad pointer
>> dereference in regulator_get_voltage().
>>
>> I confirmed that reverting this patch on top of next-20151009 gets my
>> peach-pi booting again.
>>
>> Note that the boot failure is only on exynos_defconfig, not multi_v7_defconfig.
>
> Right.  See <http://www.spinics.net/lists/arm-kernel/msg450802.html>
> for discussion.  Ulf responded to the cover letter rather than the
> individual patch.

Ah, I hadn't noticed that.  Thanks.

I guess it should be dropped by tomorrow's linux-next then.  I'll keep
an eye on it.

Kevin

[PATCH v2 4/8] mmc: dw_mmc: Use mmc_regulator_set_vqmmc in start_signal_voltage_switch

[Kevin Hilman]

On Fri, Oct 9, 2015 at 10:12 PM, Doug Anderson <dianders@chromium.org> wrote:
> Kevin,
>
> On Fri, Oct 9, 2015 at 12:06 PM, Kevin Hilman <khilman@kernel.org> wrote:
>> On Wed, Sep 30, 2015 at 4:07 PM, Heiko Stuebner <heiko@sntech.de> wrote:
>>> From: Douglas Anderson <dianders@chromium.org>
>>>
>>> We've introduced a new helper in the MMC core:
>>> mmc_regulator_set_vqmmc().  Let's use this in dw_mmc.  Using this new
>>> helper has some advantages:
>>>
>>> 1. We get the mmc_regulator_set_vqmmc() behavior of trying to match
>>>    VQMMC and VMMC when the signal voltage is 3.3V.  This ensures max
>>>    compatibility.
>>>
>>> 2. We get rid of a few more warnings when probing unsupported
>>>    voltages.
>>>
>>> 3. We get rid of some non-dw_mmc specific code in dw_mmc.
>>>
>>> Signed-off-by: Douglas Anderson <dianders@chromium.org>
>>> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
>>
>> kernelci.org found a new failure in linux-next on
>> exynos5800-peach-pi[1] and it was bisected down to this patch, which
>> has hit linux-next in the form of commit fba2e23128d0 (mmc: dw_mmc:
>> Use mmc_regulator_set_vqmmc in start_signal_voltage_switch).  Based on
>> a quick glance at the boot log[2], it looks to be a bad pointer
>> dereference in regulator_get_voltage().
>>
>> I confirmed that reverting this patch on top of next-20151009 gets my
>> peach-pi booting again.
>>
>> Note that the boot failure is only on exynos_defconfig, not multi_v7_defconfig.
>
> Right.  See <http://www.spinics.net/lists/arm-kernel/msg450802.html>
> for discussion.  Ulf responded to the cover letter rather than the
> individual patch.

Ah, I hadn't noticed that.  Thanks.

I guess it should be dropped by tomorrow's linux-next then.  I'll keep
an eye on it.

Kevin

Re: [PATCH v2 0/8] mmc: dw_mmc-rockchip: allow tuning using the clk-phase api

[Heiko Stuebner]

Am Freitag, 9. Oktober 2015, 13:11:21 schrieb Doug Anderson:
> Hi,
> 
> On Fri, Oct 9, 2015 at 10:19 AM, Doug Anderson <dianders@chromium.org> 
wrote:
> > Hi,
> > 
> > On Fri, Oct 9, 2015 at 1:10 AM, Ulf Hansson <ulf.hansson@linaro.org> 
wrote:
> >> Dough, Heiko,
> >> 
> >> This patchset seems to be causing a boot regression for
> >> exynos5800-peach-pi [1].
> >> 
> >> Apparently, the vmmc regulator doesn't exist for one of the controller
> >> but vqmmc is. This leads to the following NULL pointer exception.
> > 
> > Ugh.  Hrm, it didn't used to be possible for vmmc to be NULL.  :-/  It
> > 
> > changed in v3.16:
> >   4d1f52f mmc: core: Improve support for deferred regulators
> > 
> > ...and I didn't notice it (most of my development is done on 3.14).  :(
> > 
> > 
> > Is it sensible to have vqmmc without vmmc.  One option would be to
> > change mmc_regulator_get_supply() to not even bother trying to get
> > vqmmc if vmmc is not there.
> > 
> > Another option would be to just totally fail mmc_regulator_set_vqmmc()
> > always if vmmc is an error.  ...or assume that any bogus vmmc (either
> > vmmc is an error or regulator_get_voltage(vmmc) returns an error) is
> > 3.3V?
> > 
> > 
> > I guess the safest is to assume that bogus vmmc means 3.3V...
> 
> Ah, doh.  Just noticed Ulf's suggestion all the way at the bottom of
> the crash info, which I somehow missed before.  Yes, let's do that.
> Heiko: are you going to re-post?

definitly :-) .

I was just travelling back from ELCE today, otherwise I would've done that 
directly today. So I'm hoping to get that done tomorrow.


Heiko

[PATCH v2 0/8] mmc: dw_mmc-rockchip: allow tuning using the clk-phase api

[Heiko Stuebner]

Am Freitag, 9. Oktober 2015, 13:11:21 schrieb Doug Anderson:
> Hi,
> 
> On Fri, Oct 9, 2015 at 10:19 AM, Doug Anderson <dianders@chromium.org> 
wrote:
> > Hi,
> > 
> > On Fri, Oct 9, 2015 at 1:10 AM, Ulf Hansson <ulf.hansson@linaro.org> 
wrote:
> >> Dough, Heiko,
> >> 
> >> This patchset seems to be causing a boot regression for
> >> exynos5800-peach-pi [1].
> >> 
> >> Apparently, the vmmc regulator doesn't exist for one of the controller
> >> but vqmmc is. This leads to the following NULL pointer exception.
> > 
> > Ugh.  Hrm, it didn't used to be possible for vmmc to be NULL.  :-/  It
> > 
> > changed in v3.16:
> >   4d1f52f mmc: core: Improve support for deferred regulators
> > 
> > ...and I didn't notice it (most of my development is done on 3.14).  :(
> > 
> > 
> > Is it sensible to have vqmmc without vmmc.  One option would be to
> > change mmc_regulator_get_supply() to not even bother trying to get
> > vqmmc if vmmc is not there.
> > 
> > Another option would be to just totally fail mmc_regulator_set_vqmmc()
> > always if vmmc is an error.  ...or assume that any bogus vmmc (either
> > vmmc is an error or regulator_get_voltage(vmmc) returns an error) is
> > 3.3V?
> > 
> > 
> > I guess the safest is to assume that bogus vmmc means 3.3V...
> 
> Ah, doh.  Just noticed Ulf's suggestion all the way at the bottom of
> the crash info, which I somehow missed before.  Yes, let's do that.
> Heiko: are you going to re-post?

definitly :-) .

I was just travelling back from ELCE today, otherwise I would've done that 
directly today. So I'm hoping to get that done tomorrow.


Heiko

Re: [PATCH v2 1/8] clk: rockchip: Allow more precision for some mmc clock phases

[Michael Turquette]

Quoting Heiko Stuebner (2015-09-30 07:07:37)
> From: Douglas Anderson <dianders@chromium.org>
> =

> Because of the inexact nature of the extra MMC delay elements (it's
> not possible to keep the phase monotonic and to also make phases (mod
> 90) > 70), we previously only allowed phases (mod 90) of 22.5, 45,
> and 67.5.
> =

> But it's not the end of the world if the MMC clock phase goes
> non-monotonic.  At most we'll be 25 degrees off.  It's way better to
> test more phases to look for bad ones than to be 25 degrees off, because
> in the case of MMC really the point is to find bad phases and get as far
> asway from the as possible.  If we get to test extra phases by going
> slightly non-monotonic then that might be fine.  Worst case we would
> end up at a phases that's slight differnt than the one we wanted, but
> at least we'd still be quite far away from the a bad phase.
> =

> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> =

> Fold in more precise variance-values of 44-77 instead of 40-80.
> Fold in the actual removal of the monotonic requirement and adapt
> patch message accordingly.
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>

Looks good to me. What tree do you want this to go through?

Regards,
Mike

> ---
>  drivers/clk/rockchip/clk-mmc-phase.c | 45 ++++++++++++++++++++++++------=
------
>  1 file changed, 30 insertions(+), 15 deletions(-)
> =

> diff --git a/drivers/clk/rockchip/clk-mmc-phase.c b/drivers/clk/rockchip/=
clk-mmc-phase.c
> index 9b61342..a797d86 100644
> --- a/drivers/clk/rockchip/clk-mmc-phase.c
> +++ b/drivers/clk/rockchip/clk-mmc-phase.c
> @@ -45,8 +45,8 @@ static unsigned long rockchip_mmc_recalc(struct clk_hw =
*hw,
>  #define PSECS_PER_SEC 1000000000000LL
>  =

>  /*
> - * Each fine delay is between 40ps-80ps. Assume each fine delay is 60ps =
to
> - * simplify calculations. So 45degs could be anywhere between 33deg and =
66deg.
> + * Each fine delay is between 44ps-77ps. Assume each fine delay is 60ps =
to
> + * simplify calculations. So 45degs could be anywhere between 33deg and =
57.8deg.
>   */
>  #define ROCKCHIP_MMC_DELAY_ELEMENT_PSEC 60
>  =

> @@ -84,22 +84,37 @@ static int rockchip_mmc_set_phase(struct clk_hw *hw, =
int degrees)
>         u32 raw_value;
>         u64 delay;
>  =

> -       /* allow 22 to be 22.5 */
> -       degrees++;
> -       /* floor to 22.5 increment */
> -       degrees -=3D ((degrees) * 10 % 225) / 10;
> -
>         nineties =3D degrees / 90;
> -       /* 22.5 multiples */
> -       remainder =3D (degrees % 90) / 22;
> -
> +       remainder =3D (degrees % 90);
> +
> +       /*
> +        * Due to the inexact nature of the "fine" delay, we might
> +        * actually go non-monotonic.  We don't go _too_ monotonic
> +        * though, so we should be OK.  Here are options of how we may
> +        * work:
> +        *
> +        * Ideally we end up with:
> +        *   1.0, 2.0, ..., 69.0, 70.0, ...,  89.0, 90.0
> +        *
> +        * On one extreme (if delay is actually 44ps):
> +        *   .73, 1.5, ..., 50.6, 51.3, ...,  65.3, 90.0
> +        * The other (if delay is actually 77ps):
> +        *   1.3, 2.6, ..., 88.6. 89.8, ..., 114.0, 90
> +        *
> +        * It's possible we might make a delay that is up to 25
> +        * degrees off from what we think we're making.  That's OK
> +        * though because we should be REALLY far from any bad range.
> +        */
> +
> +       /*
> +        * Convert to delay; do a little extra work to make sure we
> +        * don't overflow 32-bit / 64-bit numbers.
> +        */
>         delay =3D PSECS_PER_SEC;
> -       do_div(delay, rate);
> -       /* / 360 / 22.5 */
> -       do_div(delay, 16);
> -       do_div(delay, ROCKCHIP_MMC_DELAY_ELEMENT_PSEC);
> -
>         delay *=3D remainder;
> +       do_div(delay, 10000);
> +       do_div(delay, (rate / 1000) * 36 * ROCKCHIP_MMC_DELAY_ELEMENT_PSE=
C);
> +
>         delay_num =3D (u8) min(delay, 255ULL);
>  =

>         raw_value =3D delay_num ? ROCKCHIP_MMC_DELAY_SEL : 0;
> -- =

> 2.5.1
>=20

Re: [PATCH v2 1/8] clk: rockchip: Allow more precision for some mmc clock phases

[Michael Turquette]

Quoting Heiko Stuebner (2015-09-30 07:07:37)
> From: Douglas Anderson <dianders@chromium.org>
> 
> Because of the inexact nature of the extra MMC delay elements (it's
> not possible to keep the phase monotonic and to also make phases (mod
> 90) > 70), we previously only allowed phases (mod 90) of 22.5, 45,
> and 67.5.
> 
> But it's not the end of the world if the MMC clock phase goes
> non-monotonic.  At most we'll be 25 degrees off.  It's way better to
> test more phases to look for bad ones than to be 25 degrees off, because
> in the case of MMC really the point is to find bad phases and get as far
> asway from the as possible.  If we get to test extra phases by going
> slightly non-monotonic then that might be fine.  Worst case we would
> end up at a phases that's slight differnt than the one we wanted, but
> at least we'd still be quite far away from the a bad phase.
> 
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> 
> Fold in more precise variance-values of 44-77 instead of 40-80.
> Fold in the actual removal of the monotonic requirement and adapt
> patch message accordingly.
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>

Looks good to me. What tree do you want this to go through?

Regards,
Mike

> ---
>  drivers/clk/rockchip/clk-mmc-phase.c | 45 ++++++++++++++++++++++++------------
>  1 file changed, 30 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/clk/rockchip/clk-mmc-phase.c b/drivers/clk/rockchip/clk-mmc-phase.c
> index 9b61342..a797d86 100644
> --- a/drivers/clk/rockchip/clk-mmc-phase.c
> +++ b/drivers/clk/rockchip/clk-mmc-phase.c
> @@ -45,8 +45,8 @@ static unsigned long rockchip_mmc_recalc(struct clk_hw *hw,
>  #define PSECS_PER_SEC 1000000000000LL
>  
>  /*
> - * Each fine delay is between 40ps-80ps. Assume each fine delay is 60ps to
> - * simplify calculations. So 45degs could be anywhere between 33deg and 66deg.
> + * Each fine delay is between 44ps-77ps. Assume each fine delay is 60ps to
> + * simplify calculations. So 45degs could be anywhere between 33deg and 57.8deg.
>   */
>  #define ROCKCHIP_MMC_DELAY_ELEMENT_PSEC 60
>  
> @@ -84,22 +84,37 @@ static int rockchip_mmc_set_phase(struct clk_hw *hw, int degrees)
>         u32 raw_value;
>         u64 delay;
>  
> -       /* allow 22 to be 22.5 */
> -       degrees++;
> -       /* floor to 22.5 increment */
> -       degrees -= ((degrees) * 10 % 225) / 10;
> -
>         nineties = degrees / 90;
> -       /* 22.5 multiples */
> -       remainder = (degrees % 90) / 22;
> -
> +       remainder = (degrees % 90);
> +
> +       /*
> +        * Due to the inexact nature of the "fine" delay, we might
> +        * actually go non-monotonic.  We don't go _too_ monotonic
> +        * though, so we should be OK.  Here are options of how we may
> +        * work:
> +        *
> +        * Ideally we end up with:
> +        *   1.0, 2.0, ..., 69.0, 70.0, ...,  89.0, 90.0
> +        *
> +        * On one extreme (if delay is actually 44ps):
> +        *   .73, 1.5, ..., 50.6, 51.3, ...,  65.3, 90.0
> +        * The other (if delay is actually 77ps):
> +        *   1.3, 2.6, ..., 88.6. 89.8, ..., 114.0, 90
> +        *
> +        * It's possible we might make a delay that is up to 25
> +        * degrees off from what we think we're making.  That's OK
> +        * though because we should be REALLY far from any bad range.
> +        */
> +
> +       /*
> +        * Convert to delay; do a little extra work to make sure we
> +        * don't overflow 32-bit / 64-bit numbers.
> +        */
>         delay = PSECS_PER_SEC;
> -       do_div(delay, rate);
> -       /* / 360 / 22.5 */
> -       do_div(delay, 16);
> -       do_div(delay, ROCKCHIP_MMC_DELAY_ELEMENT_PSEC);
> -
>         delay *= remainder;
> +       do_div(delay, 10000);
> +       do_div(delay, (rate / 1000) * 36 * ROCKCHIP_MMC_DELAY_ELEMENT_PSEC);
> +
>         delay_num = (u8) min(delay, 255ULL);
>  
>         raw_value = delay_num ? ROCKCHIP_MMC_DELAY_SEL : 0;
> -- 
> 2.5.1
> 

[PATCH v2 1/8] clk: rockchip: Allow more precision for some mmc clock phases

[Michael Turquette]

Quoting Heiko Stuebner (2015-09-30 07:07:37)
> From: Douglas Anderson <dianders@chromium.org>
> 
> Because of the inexact nature of the extra MMC delay elements (it's
> not possible to keep the phase monotonic and to also make phases (mod
> 90) > 70), we previously only allowed phases (mod 90) of 22.5, 45,
> and 67.5.
> 
> But it's not the end of the world if the MMC clock phase goes
> non-monotonic.  At most we'll be 25 degrees off.  It's way better to
> test more phases to look for bad ones than to be 25 degrees off, because
> in the case of MMC really the point is to find bad phases and get as far
> asway from the as possible.  If we get to test extra phases by going
> slightly non-monotonic then that might be fine.  Worst case we would
> end up at a phases that's slight differnt than the one we wanted, but
> at least we'd still be quite far away from the a bad phase.
> 
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> 
> Fold in more precise variance-values of 44-77 instead of 40-80.
> Fold in the actual removal of the monotonic requirement and adapt
> patch message accordingly.
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>

Looks good to me. What tree do you want this to go through?

Regards,
Mike

> ---
>  drivers/clk/rockchip/clk-mmc-phase.c | 45 ++++++++++++++++++++++++------------
>  1 file changed, 30 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/clk/rockchip/clk-mmc-phase.c b/drivers/clk/rockchip/clk-mmc-phase.c
> index 9b61342..a797d86 100644
> --- a/drivers/clk/rockchip/clk-mmc-phase.c
> +++ b/drivers/clk/rockchip/clk-mmc-phase.c
> @@ -45,8 +45,8 @@ static unsigned long rockchip_mmc_recalc(struct clk_hw *hw,
>  #define PSECS_PER_SEC 1000000000000LL
>  
>  /*
> - * Each fine delay is between 40ps-80ps. Assume each fine delay is 60ps to
> - * simplify calculations. So 45degs could be anywhere between 33deg and 66deg.
> + * Each fine delay is between 44ps-77ps. Assume each fine delay is 60ps to
> + * simplify calculations. So 45degs could be anywhere between 33deg and 57.8deg.
>   */
>  #define ROCKCHIP_MMC_DELAY_ELEMENT_PSEC 60
>  
> @@ -84,22 +84,37 @@ static int rockchip_mmc_set_phase(struct clk_hw *hw, int degrees)
>         u32 raw_value;
>         u64 delay;
>  
> -       /* allow 22 to be 22.5 */
> -       degrees++;
> -       /* floor to 22.5 increment */
> -       degrees -= ((degrees) * 10 % 225) / 10;
> -
>         nineties = degrees / 90;
> -       /* 22.5 multiples */
> -       remainder = (degrees % 90) / 22;
> -
> +       remainder = (degrees % 90);
> +
> +       /*
> +        * Due to the inexact nature of the "fine" delay, we might
> +        * actually go non-monotonic.  We don't go _too_ monotonic
> +        * though, so we should be OK.  Here are options of how we may
> +        * work:
> +        *
> +        * Ideally we end up with:
> +        *   1.0, 2.0, ..., 69.0, 70.0, ...,  89.0, 90.0
> +        *
> +        * On one extreme (if delay is actually 44ps):
> +        *   .73, 1.5, ..., 50.6, 51.3, ...,  65.3, 90.0
> +        * The other (if delay is actually 77ps):
> +        *   1.3, 2.6, ..., 88.6. 89.8, ..., 114.0, 90
> +        *
> +        * It's possible we might make a delay that is up to 25
> +        * degrees off from what we think we're making.  That's OK
> +        * though because we should be REALLY far from any bad range.
> +        */
> +
> +       /*
> +        * Convert to delay; do a little extra work to make sure we
> +        * don't overflow 32-bit / 64-bit numbers.
> +        */
>         delay = PSECS_PER_SEC;
> -       do_div(delay, rate);
> -       /* / 360 / 22.5 */
> -       do_div(delay, 16);
> -       do_div(delay, ROCKCHIP_MMC_DELAY_ELEMENT_PSEC);
> -
>         delay *= remainder;
> +       do_div(delay, 10000);
> +       do_div(delay, (rate / 1000) * 36 * ROCKCHIP_MMC_DELAY_ELEMENT_PSEC);
> +
>         delay_num = (u8) min(delay, 255ULL);
>  
>         raw_value = delay_num ? ROCKCHIP_MMC_DELAY_SEL : 0;
> -- 
> 2.5.1
> 

Re: [PATCH v2 2/8] clk: rockchip: Make calculations use rounding

[Michael Turquette]

Quoting Heiko Stuebner (2015-09-30 07:07:38)
> From: Douglas Anderson <dianders@chromium.org>
> =

> Let's use DIV_ROUND_CLOSEST for rounding, not just truncating
> division.  This lets us get closer to the right rate.
> =

> Before this:
>   set_phase(86) delay_nums=3D26 reg[0xf000420c]=3D0x468 actual_degrees=3D=
83
>   set_phase(89) delay_nums=3D27 reg[0xf000420c]=3D0x46c actual_degrees=3D=
86
> =

> After this:
>   set_phase(86) delay_nums=3D27 reg[0xf000420c]=3D0x46c actual_degrees=3D=
86
>   set_phase(89) delay_nums=3D28 reg[0xf000420c]=3D0x470 actual_degrees=3D=
90
> =

> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>

Looks good to me. What tree do you want this to go through?

Regards,
Mike

> ---
>  drivers/clk/rockchip/clk-mmc-phase.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
> =

> diff --git a/drivers/clk/rockchip/clk-mmc-phase.c b/drivers/clk/rockchip/=
clk-mmc-phase.c
> index a797d86..bc24e5a 100644
> --- a/drivers/clk/rockchip/clk-mmc-phase.c
> +++ b/drivers/clk/rockchip/clk-mmc-phase.c
> @@ -69,7 +69,7 @@ static int rockchip_mmc_get_phase(struct clk_hw *hw)
>  =

>                 delay_num =3D (raw_value & ROCKCHIP_MMC_DELAYNUM_MASK);
>                 delay_num >>=3D ROCKCHIP_MMC_DELAYNUM_OFFSET;
> -               degrees +=3D delay_num * factor / 10000;
> +               degrees +=3D DIV_ROUND_CLOSEST(delay_num * factor, 10000);
>         }
>  =

>         return degrees % 360;
> @@ -82,7 +82,7 @@ static int rockchip_mmc_set_phase(struct clk_hw *hw, in=
t degrees)
>         u8 nineties, remainder;
>         u8 delay_num;
>         u32 raw_value;
> -       u64 delay;
> +       u32 delay;
>  =

>         nineties =3D degrees / 90;
>         remainder =3D (degrees % 90);
> @@ -110,12 +110,13 @@ static int rockchip_mmc_set_phase(struct clk_hw *hw=
, int degrees)
>          * Convert to delay; do a little extra work to make sure we
>          * don't overflow 32-bit / 64-bit numbers.
>          */
> -       delay =3D PSECS_PER_SEC;
> +       delay =3D 10000000; /* PSECS_PER_SEC / 10000 / 10 */
>         delay *=3D remainder;
> -       do_div(delay, 10000);
> -       do_div(delay, (rate / 1000) * 36 * ROCKCHIP_MMC_DELAY_ELEMENT_PSE=
C);
> +       delay =3D DIV_ROUND_CLOSEST(delay,
> +                       (rate / 1000) * 36 *
> +                               (ROCKCHIP_MMC_DELAY_ELEMENT_PSEC / 10));
>  =

> -       delay_num =3D (u8) min(delay, 255ULL);
> +       delay_num =3D (u8) min_t(u32, delay, 255);
>  =

>         raw_value =3D delay_num ? ROCKCHIP_MMC_DELAY_SEL : 0;
>         raw_value |=3D delay_num << ROCKCHIP_MMC_DELAYNUM_OFFSET;
> -- =

> 2.5.1
>=20

Re: [PATCH v2 2/8] clk: rockchip: Make calculations use rounding

[Michael Turquette]

Quoting Heiko Stuebner (2015-09-30 07:07:38)
> From: Douglas Anderson <dianders@chromium.org>
> 
> Let's use DIV_ROUND_CLOSEST for rounding, not just truncating
> division.  This lets us get closer to the right rate.
> 
> Before this:
>   set_phase(86) delay_nums=26 reg[0xf000420c]=0x468 actual_degrees=83
>   set_phase(89) delay_nums=27 reg[0xf000420c]=0x46c actual_degrees=86
> 
> After this:
>   set_phase(86) delay_nums=27 reg[0xf000420c]=0x46c actual_degrees=86
>   set_phase(89) delay_nums=28 reg[0xf000420c]=0x470 actual_degrees=90
> 
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>

Looks good to me. What tree do you want this to go through?

Regards,
Mike

> ---
>  drivers/clk/rockchip/clk-mmc-phase.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/clk/rockchip/clk-mmc-phase.c b/drivers/clk/rockchip/clk-mmc-phase.c
> index a797d86..bc24e5a 100644
> --- a/drivers/clk/rockchip/clk-mmc-phase.c
> +++ b/drivers/clk/rockchip/clk-mmc-phase.c
> @@ -69,7 +69,7 @@ static int rockchip_mmc_get_phase(struct clk_hw *hw)
>  
>                 delay_num = (raw_value & ROCKCHIP_MMC_DELAYNUM_MASK);
>                 delay_num >>= ROCKCHIP_MMC_DELAYNUM_OFFSET;
> -               degrees += delay_num * factor / 10000;
> +               degrees += DIV_ROUND_CLOSEST(delay_num * factor, 10000);
>         }
>  
>         return degrees % 360;
> @@ -82,7 +82,7 @@ static int rockchip_mmc_set_phase(struct clk_hw *hw, int degrees)
>         u8 nineties, remainder;
>         u8 delay_num;
>         u32 raw_value;
> -       u64 delay;
> +       u32 delay;
>  
>         nineties = degrees / 90;
>         remainder = (degrees % 90);
> @@ -110,12 +110,13 @@ static int rockchip_mmc_set_phase(struct clk_hw *hw, int degrees)
>          * Convert to delay; do a little extra work to make sure we
>          * don't overflow 32-bit / 64-bit numbers.
>          */
> -       delay = PSECS_PER_SEC;
> +       delay = 10000000; /* PSECS_PER_SEC / 10000 / 10 */
>         delay *= remainder;
> -       do_div(delay, 10000);
> -       do_div(delay, (rate / 1000) * 36 * ROCKCHIP_MMC_DELAY_ELEMENT_PSEC);
> +       delay = DIV_ROUND_CLOSEST(delay,
> +                       (rate / 1000) * 36 *
> +                               (ROCKCHIP_MMC_DELAY_ELEMENT_PSEC / 10));
>  
> -       delay_num = (u8) min(delay, 255ULL);
> +       delay_num = (u8) min_t(u32, delay, 255);
>  
>         raw_value = delay_num ? ROCKCHIP_MMC_DELAY_SEL : 0;
>         raw_value |= delay_num << ROCKCHIP_MMC_DELAYNUM_OFFSET;
> -- 
> 2.5.1
> 

[PATCH v2 2/8] clk: rockchip: Make calculations use rounding

[Michael Turquette]

Quoting Heiko Stuebner (2015-09-30 07:07:38)
> From: Douglas Anderson <dianders@chromium.org>
> 
> Let's use DIV_ROUND_CLOSEST for rounding, not just truncating
> division.  This lets us get closer to the right rate.
> 
> Before this:
>   set_phase(86) delay_nums=26 reg[0xf000420c]=0x468 actual_degrees=83
>   set_phase(89) delay_nums=27 reg[0xf000420c]=0x46c actual_degrees=86
> 
> After this:
>   set_phase(86) delay_nums=27 reg[0xf000420c]=0x46c actual_degrees=86
>   set_phase(89) delay_nums=28 reg[0xf000420c]=0x470 actual_degrees=90
> 
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>

Looks good to me. What tree do you want this to go through?

Regards,
Mike

> ---
>  drivers/clk/rockchip/clk-mmc-phase.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/clk/rockchip/clk-mmc-phase.c b/drivers/clk/rockchip/clk-mmc-phase.c
> index a797d86..bc24e5a 100644
> --- a/drivers/clk/rockchip/clk-mmc-phase.c
> +++ b/drivers/clk/rockchip/clk-mmc-phase.c
> @@ -69,7 +69,7 @@ static int rockchip_mmc_get_phase(struct clk_hw *hw)
>  
>                 delay_num = (raw_value & ROCKCHIP_MMC_DELAYNUM_MASK);
>                 delay_num >>= ROCKCHIP_MMC_DELAYNUM_OFFSET;
> -               degrees += delay_num * factor / 10000;
> +               degrees += DIV_ROUND_CLOSEST(delay_num * factor, 10000);
>         }
>  
>         return degrees % 360;
> @@ -82,7 +82,7 @@ static int rockchip_mmc_set_phase(struct clk_hw *hw, int degrees)
>         u8 nineties, remainder;
>         u8 delay_num;
>         u32 raw_value;
> -       u64 delay;
> +       u32 delay;
>  
>         nineties = degrees / 90;
>         remainder = (degrees % 90);
> @@ -110,12 +110,13 @@ static int rockchip_mmc_set_phase(struct clk_hw *hw, int degrees)
>          * Convert to delay; do a little extra work to make sure we
>          * don't overflow 32-bit / 64-bit numbers.
>          */
> -       delay = PSECS_PER_SEC;
> +       delay = 10000000; /* PSECS_PER_SEC / 10000 / 10 */
>         delay *= remainder;
> -       do_div(delay, 10000);
> -       do_div(delay, (rate / 1000) * 36 * ROCKCHIP_MMC_DELAY_ELEMENT_PSEC);
> +       delay = DIV_ROUND_CLOSEST(delay,
> +                       (rate / 1000) * 36 *
> +                               (ROCKCHIP_MMC_DELAY_ELEMENT_PSEC / 10));
>  
> -       delay_num = (u8) min(delay, 255ULL);
> +       delay_num = (u8) min_t(u32, delay, 255);
>  
>         raw_value = delay_num ? ROCKCHIP_MMC_DELAY_SEL : 0;
>         raw_value |= delay_num << ROCKCHIP_MMC_DELAYNUM_OFFSET;
> -- 
> 2.5.1
> 

Re: [PATCH v2 1/8] clk: rockchip: Allow more precision for some mmc clock phases

[Heiko Stübner]

Hi Mike,

Am Donnerstag, 22. Oktober 2015, 05:03:26 schrieb Michael Turquette:
> Quoting Heiko Stuebner (2015-09-30 07:07:37)
> 
> > From: Douglas Anderson <dianders@chromium.org>
> > 
> > Because of the inexact nature of the extra MMC delay elements (it's
> > not possible to keep the phase monotonic and to also make phases (mod
> > 90) > 70), we previously only allowed phases (mod 90) of 22.5, 45,
> > and 67.5.
> > 
> > But it's not the end of the world if the MMC clock phase goes
> > non-monotonic.  At most we'll be 25 degrees off.  It's way better to
> > test more phases to look for bad ones than to be 25 degrees off, because
> > in the case of MMC really the point is to find bad phases and get as far
> > asway from the as possible.  If we get to test extra phases by going
> > slightly non-monotonic then that might be fine.  Worst case we would
> > end up at a phases that's slight differnt than the one we wanted, but
> > at least we'd still be quite far away from the a bad phase.
> > 
> > Signed-off-by: Douglas Anderson <dianders@chromium.org>
> > 
> > Fold in more precise variance-values of 44-77 instead of 40-80.
> > Fold in the actual removal of the monotonic requirement and adapt
> > patch message accordingly.
> > Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> 
> Looks good to me. What tree do you want this to go through?

we got already an Ack from Stephen [in the thread of patch3] for the clock 
patches and they are already in the mmc.tree [0].


Heiko


[0] 
https://git.linaro.org/people/ulf.hansson/mmc.git/commit/6cf04362a13de8cdb59b85b8dec1d53f90307dd1

[PATCH v2 1/8] clk: rockchip: Allow more precision for some mmc clock phases

[Heiko Stübner]

Hi Mike,

Am Donnerstag, 22. Oktober 2015, 05:03:26 schrieb Michael Turquette:
> Quoting Heiko Stuebner (2015-09-30 07:07:37)
> 
> > From: Douglas Anderson <dianders@chromium.org>
> > 
> > Because of the inexact nature of the extra MMC delay elements (it's
> > not possible to keep the phase monotonic and to also make phases (mod
> > 90) > 70), we previously only allowed phases (mod 90) of 22.5, 45,
> > and 67.5.
> > 
> > But it's not the end of the world if the MMC clock phase goes
> > non-monotonic.  At most we'll be 25 degrees off.  It's way better to
> > test more phases to look for bad ones than to be 25 degrees off, because
> > in the case of MMC really the point is to find bad phases and get as far
> > asway from the as possible.  If we get to test extra phases by going
> > slightly non-monotonic then that might be fine.  Worst case we would
> > end up at a phases that's slight differnt than the one we wanted, but
> > at least we'd still be quite far away from the a bad phase.
> > 
> > Signed-off-by: Douglas Anderson <dianders@chromium.org>
> > 
> > Fold in more precise variance-values of 44-77 instead of 40-80.
> > Fold in the actual removal of the monotonic requirement and adapt
> > patch message accordingly.
> > Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> 
> Looks good to me. What tree do you want this to go through?

we got already an Ack from Stephen [in the thread of patch3] for the clock 
patches and they are already in the mmc.tree [0].


Heiko


[0] 
https://git.linaro.org/people/ulf.hansson/mmc.git/commit/6cf04362a13de8cdb59b85b8dec1d53f90307dd1