[v6,4/4] mmc: sdhci-of-arasan: Enable UHS-1 support for Keem Bay SOC
diff mbox series

Message ID 20201202150205.20150-5-muhammad.husaini.zulkifli@intel.com
State New, archived
Headers show
Series
  • mmc: sdhci-of-arasan: Enable UHS-1 support for Keem Bay SOC
Related show

Commit Message

Zulkifli, Muhammad Husaini Dec. 2, 2020, 3:02 p.m. UTC
From: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com>

Keem Bay SOC can support dual voltage operations for GPIO SD Pins to
either 1.8V or 3.3V for bus IO line power. In order to operate the GPIOs
line for Clk,Cmd and Data on Keem Bay Hardware, it is important to
configure the supplied voltage applied to their I/O Rail and the output
of the i2c expander pin. Final Voltage applied on the GPIOs Line are
dependent by both supplied voltage rail and expander pin output as it is
been set after passing through the voltage sense resistor.

The Keem Bay HW is somewhat unique in the way of how IO bus line voltage
are been controlled. Output of the Expander pins is been configured using
regulator. Voltage rail output is being configured using
keembay_io_rail_supplied_voltage() API in the sdhci driver directly.
Pin control based implementation becomes problematic to control the
voltage rail due to the base address of Always On Register is
different from the base address of GPIO(Pinctrl). Thus, there is
no way to control the I/O Rail using GPIO Pad configuration.
On the other hand, using ARM SMC (Secure Monitor Call) directly from
pin control driver for the sake of implement it as pin control model
is not a good approach.

This patch was tested with Keem Bay EVM.

Signed-off-by: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Reviewed-by: Adrian Hunter <adrian.hunter@intel.com>
Acked-by: Michal Simek <michal.simek@xilinx.com>
---
 drivers/mmc/host/sdhci-of-arasan.c | 244 +++++++++++++++++++++++++++++
 1 file changed, 244 insertions(+)

Comments

Linus Walleij Dec. 2, 2020, 6:54 p.m. UTC | #1
Hi Muhammad,

thanks for your patch!

On Wed, Dec 2, 2020 at 8:04 AM <muhammad.husaini.zulkifli@intel.com> wrote:

> Keem Bay SOC can support dual voltage operations for GPIO SD Pins to
> either 1.8V or 3.3V for bus IO line power. In order to operate the GPIOs
> line for Clk,Cmd and Data on Keem Bay Hardware, it is important to
> configure the supplied voltage applied to their I/O Rail and the output
> of the i2c expander pin. Final Voltage applied on the GPIOs Line are
> dependent by both supplied voltage rail and expander pin output as it is
> been set after passing through the voltage sense resistor.

I think I understand this part.

> The Keem Bay HW is somewhat unique in the way of how IO bus line voltage
> are been controlled. Output of the Expander pins is been configured using
> regulator.

That much is clear.

> Voltage rail output is being configured using
> keembay_io_rail_supplied_voltage() API in the sdhci driver directly.

And that is an SMC call like that:

+static inline int keembay_io_rail_supplied_voltage(int volt)
+{
+       struct arm_smccc_res res;
+
+       arm_smccc_1_1_invoke(ARM_SMCCC_SIP_KEEMBAY_SET_SD_VOLTAGE, volt, &res);
+       if ((int)res.a0 < 0)
+               return -EINVAL;
+
+       return 0;

That can set the voltage by calling into the Arm secure world I guess?

> Pin control based implementation becomes problematic to control the
> voltage rail due to the base address of Always On Register is
> different fromThe driver does not have to be in the the base address of GPIO(Pinctrl). Thus, there is
> no way to control the I/O Rail using GPIO Pad configuration.

I don't see why this would be pin control related, and that is as
you point out leading to some confused discussions here.

We do have something like this generic pin config:

 * @PIN_CONFIG_POWER_SOURCE: if the pin can select between different power
 *      supplies, the argument to this parameter (on a custom format) tells
 *      the driver which alternative power source to use.

But it's ... yeah. It usually has a very specific purpose of selecting
one of two available voltage rails inside the SoC. And it needs to
apply to one pin or pin group. Also it kind of implies that those
voltages are always on.

As you say:

> From the Databook itself with additional confirmation from
> Keem Bay HW SOC Design Architect,
> there is no direct control of these AON register bits from
> GPIO pads.

The keembay_io_rail_supplied_voltage() more resembles a
selector (choose one on a menu) voltage regulator to me
if anything.

> On the other hand, using ARM SMC (Secure Monitor Call) directly from
> pin control driver for the sake of implement it as pin control model
> is not a good approach.

Yeah it has to be called from somewhere, if you want an abstraction
to make the driver neutral to any machine, then use a
selector regulator. It can be placed
anywhere in the kernel as long as you can reference it.

The register is called (according to the code) AON_CGF1
(really? not AON_CFG1?) and the "ON" part in "AON"  makes
it sound like "analog ON" implying this is something that can be
turned on/off and configured into two voltages and it has been
wrapped in these custom SMCCs by a secure world developer
(right?)

If it should use any abstraction it should be a selector regulator
IMO and while that may seem overengineered it adds something
because regulators are used in  the MMC subsystem for vdd
and vqmmc because we are handling the OCR mask with that
and it can support any amount of present and future
voltages for signal levels with that as well. Any future changes
to how the different signal voltages are set or which voltages
exist can then be done in that regulator driver.

Just my €0.01...

Yours,
Linus Walleij
Zulkifli, Muhammad Husaini Dec. 3, 2020, 7:10 a.m. UTC | #2
Hi Linus,

Thanks for your input. I replied inline.

>-----Original Message-----
>From: Linus Walleij <linus.walleij@linaro.org>
>Sent: Thursday, December 3, 2020 2:55 AM
>To: Zulkifli, Muhammad Husaini <muhammad.husaini.zulkifli@intel.com>
>Cc: Ulf Hansson <ulf.hansson@linaro.org>; Hunter, Adrian
><adrian.hunter@intel.com>; Michal Simek <michal.simek@xilinx.com>; linux-
>mmc <linux-mmc@vger.kernel.org>; Linux ARM <linux-arm-
>kernel@lists.infradead.org>; linux-kernel@vger.kernel.org; Shevchenko,
>Andriy <andriy.shevchenko@intel.com>; Raja Subramanian, Lakshmi Bai
><lakshmi.bai.raja.subramanian@intel.com>; Wan Mohamad, Wan Ahmad
>Zainie <wan.ahmad.zainie.wan.mohamad@intel.com>; Mark Gross
><mgross@linux.intel.com>
>Subject: Re: [PATCH v6 4/4] mmc: sdhci-of-arasan: Enable UHS-1 support for
>Keem Bay SOC
>
>Hi Muhammad,
>
>thanks for your patch!
>
>On Wed, Dec 2, 2020 at 8:04 AM <muhammad.husaini.zulkifli@intel.com>
>wrote:
>
>> Keem Bay SOC can support dual voltage operations for GPIO SD Pins to
>> either 1.8V or 3.3V for bus IO line power. In order to operate the
>> GPIOs line for Clk,Cmd and Data on Keem Bay Hardware, it is important
>> to configure the supplied voltage applied to their I/O Rail and the
>> output of the i2c expander pin. Final Voltage applied on the GPIOs
>> Line are dependent by both supplied voltage rail and expander pin
>> output as it is been set after passing through the voltage sense resistor.
>
>I think I understand this part.
>
>> The Keem Bay HW is somewhat unique in the way of how IO bus line
>> voltage are been controlled. Output of the Expander pins is been
>> configured using regulator.
>
>That much is clear.
>
>> Voltage rail output is being configured using
>> keembay_io_rail_supplied_voltage() API in the sdhci driver directly.
>
>And that is an SMC call like that:
>
>+static inline int keembay_io_rail_supplied_voltage(int volt) {
>+       struct arm_smccc_res res;
>+
>+
>arm_smccc_1_1_invoke(ARM_SMCCC_SIP_KEEMBAY_SET_SD_VOLTAGE,
>volt, &res);
>+       if ((int)res.a0 < 0)
>+               return -EINVAL;
>+
>+       return 0;
>
>That can set the voltage by calling into the Arm secure world I guess?

Yes calling the Arm SMCC . 
The selection of the supply voltage applied to the rail depends on the AON_CFG1[9] .
Setting Bit9  will change the voltage rail supply.
Set Bit 9 = 1 // Operation voltage is 1.8V 
Set Bit 9 = 0 // Operation voltage is 3.3V

>
>> Pin control based implementation becomes problematic to control the
>> voltage rail due to the base address of Always On Register is
>> different fromThe driver does not have to be in the the base address
>> of GPIO(Pinctrl). Thus, there is no way to control the I/O Rail using GPIO
>Pad configuration.
>
>I don't see why this would be pin control related, and that is as you point out
>leading to some confused discussions here.

This is not related to a pin control. I pointed out this in the commit description to 
indicate that for Keem Bay HW to modelling as pinctrl to call the SMCC Arm is not 
a good approach.

>
>We do have something like this generic pin config:
>
> * @PIN_CONFIG_POWER_SOURCE: if the pin can select between different
>power
> *      supplies, the argument to this parameter (on a custom format) tells
> *      the driver which alternative power source to use.
>
>But it's ... yeah. It usually has a very specific purpose of selecting one of two
>available voltage rails inside the SoC. And it needs to apply to one pin or pin
>group. Also it kind of implies that those voltages are always on.
>

For Keembay HW, we could not apply on any pins because we do not have a 
direct access to control this pad. That is why, currently I configure this voltage rail 
supply through the SMCCs Arm wrapper. 

Unless if we fake some pin example GPIO32-37 for SD, which using "*pinconf_set" pointed
to config param of PIN_CONFIG_POWER_SOURCE, then call the keembay_io_rail_supplied_voltage()
to change the IO rail voltage feeding.
By means calling keembay_io_rail_supplied_voltage(), we call ARM SMCC in this case condition ?
I was referring to this https://elixir.bootlin.com/linux/v5.10-rc6/source/drivers/pinctrl/renesas/pinctrl.c#L706 

>As you say:
>
>> From the Databook itself with additional confirmation from Keem Bay HW
>> SOC Design Architect, there is no direct control of these AON register
>> bits from GPIO pads.
>
>The keembay_io_rail_supplied_voltage() more resembles a selector (choose
>one on a menu) voltage regulator to me if anything.

Keem Bay HW SoC does not have its own voltage regulator for sd card.
Final IO line voltage depends on supplied voltage applied to their I/O 
Rail and the output of the i2c expander pin which go into power mux.

>
>> On the other hand, using ARM SMC (Secure Monitor Call) directly from
>> pin control driver for the sake of implement it as pin control model
>> is not a good approach.
>
>Yeah it has to be called from somewhere, if you want an abstraction to make
>the driver neutral to any machine, then use a selector regulator. It can be
>placed anywhere in the kernel as long as you can reference it.

Sorry. I am not really aware of selector regulator. Can you point me to any references
for this? 

>
>The register is called (according to the code) AON_CGF1 (really? not
>AON_CFG1?) and the "ON" part in "AON"  makes it sound like "analog ON"
>implying this is something that can be turned on/off and configured into two
>voltages and it has been wrapped in these custom SMCCs by a secure world
>developer
>(right?)

It is Always On Power Domain Register. Yes it's been wrapped with SMCCC call 
due to secure register. By default voltage rail feed is 3.3v.

>
>If it should use any abstraction it should be a selector regulator IMO and
>while that may seem overengineered it adds something because regulators
>are used in  the MMC subsystem for vdd and vqmmc because we are handling
>the OCR mask with that and it can support any amount of present and future
>voltages for signal levels with that as well. Any future changes to how the
>different signal voltages are set or which voltages exist can then be done in
>that regulator driver.

This is limitation of Keem Bay HW and I would say Keem Bay HW is somewhat 
unique in the way of handling the IO bus line voltage.
SDcard does not have its own voltage regulator.
I created one function sdhci_arasan_keembay_io_line_supply_operation() in sdhci-of-arasan.c  
to handle the vqmmc(io line supply operation) specific for Keem Bay SoC.

For Keem Bay, to actually modelling this as regulator ,for vqmmc, , we need to handle 2 things:
1) Output expander pins : using gpio regulator
2) voltage rail : call keembay_io_rail_supplied_voltage() to handle the SMCC Arm.

Other hardware might not need this as they might easily configure the vqmmc 
hooked up to regulator.
 
IMHO, we do not need to overengineered it to add custom selector 
regulator just to suit this Keem Bay HW design.

>
>Just my €0.01...
>
>Yours,
>Linus Walleij
Andy Shevchenko Dec. 3, 2020, 8:12 a.m. UTC | #3
On Thu, Dec 03, 2020 at 09:10:14AM +0200, Zulkifli, Muhammad Husaini wrote:
> >From: Linus Walleij <linus.walleij@linaro.org>
> >Sent: Thursday, December 3, 2020 2:55 AM
> >On Wed, Dec 2, 2020 at 8:04 AM <muhammad.husaini.zulkifli@intel.com>
> >wrote:

...

> >If it should use any abstraction it should be a selector regulator IMO and
> >while that may seem overengineered it adds something because regulators
> >are used in  the MMC subsystem for vdd and vqmmc because we are handling
> >the OCR mask with that and it can support any amount of present and future
> >voltages for signal levels with that as well. Any future changes to how the
> >different signal voltages are set or which voltages exist can then be done in
> >that regulator driver.
> 
> This is limitation of Keem Bay HW and I would say Keem Bay HW is somewhat
> unique in the way of handling the IO bus line voltage.
> SDcard does not have its own voltage regulator.
> I created one function sdhci_arasan_keembay_io_line_supply_operation() in sdhci-of-arasan.c
> to handle the vqmmc(io line supply operation) specific for Keem Bay SoC.
> 
> For Keem Bay, to actually modelling this as regulator ,for vqmmc, , we need to handle 2 things:
> 1) Output expander pins : using gpio regulator
> 2) voltage rail : call keembay_io_rail_supplied_voltage() to handle the SMCC Arm.
> 
> Other hardware might not need this as they might easily configure the vqmmc
> hooked up to regulator.
> 
> IMHO, we do not need to overengineered it to add custom selector
> regulator just to suit this Keem Bay HW design.

I guess Linus has a point. If it can be abstracted as selector regulator it
will suits generic approach in the MMC code.

And what is the problem to have two or more regulators? Or regulator hierarchy?
Linus Walleij Dec. 5, 2020, 11:01 p.m. UTC | #4
On Thu, Dec 3, 2020 at 8:10 AM Zulkifli, Muhammad Husaini
<muhammad.husaini.zulkifli@intel.com> wrote:

> >Yeah it has to be called from somewhere, if you want an abstraction to make
> >the driver neutral to any machine, then use a selector regulator. It can be
> >placed anywhere in the kernel as long as you can reference it.
>
> Sorry. I am not really aware of selector regulator. Can you point me to any references
> for this?

It is part of the regulator subsystem and the standard framework
there to handle regulators with an enumerable number of
specific voltage levels.

> IMHO, we do not need to overengineered it to add custom selector
> regulator just to suit this Keem Bay HW design.

That can be said about a lot of things we model with vqmmc.
Using standard abstractions makes things easier for maintainers.
We mostly design abstractions for maintenance not for the simplest way
to set bits in registers.

Yours,
Linus Walleij
Zulkifli, Muhammad Husaini Dec. 12, 2020, 2:06 p.m. UTC | #5
Hi Linus,

>-----Original Message-----
>From: Linus Walleij <linus.walleij@linaro.org>
>Sent: Sunday, December 6, 2020 7:02 AM
>To: Zulkifli, Muhammad Husaini <muhammad.husaini.zulkifli@intel.com>
>Cc: Ulf Hansson <ulf.hansson@linaro.org>; Hunter, Adrian
><adrian.hunter@intel.com>; Michal Simek <michal.simek@xilinx.com>; linux-
>mmc <linux-mmc@vger.kernel.org>; Linux ARM <linux-arm-
>kernel@lists.infradead.org>; linux-kernel@vger.kernel.org; Shevchenko,
>Andriy <andriy.shevchenko@intel.com>; Raja Subramanian, Lakshmi Bai
><lakshmi.bai.raja.subramanian@intel.com>; Wan Mohamad, Wan Ahmad
>Zainie <wan.ahmad.zainie.wan.mohamad@intel.com>; Mark Gross
><mgross@linux.intel.com>
>Subject: Re: [PATCH v6 4/4] mmc: sdhci-of-arasan: Enable UHS-1 support for
>Keem Bay SOC
>
>On Thu, Dec 3, 2020 at 8:10 AM Zulkifli, Muhammad Husaini
><muhammad.husaini.zulkifli@intel.com> wrote:
>
>> >Yeah it has to be called from somewhere, if you want an abstraction
>> >to make the driver neutral to any machine, then use a selector
>> >regulator. It can be placed anywhere in the kernel as long as you can
>reference it.
>>
>> Sorry. I am not really aware of selector regulator. Can you point me
>> to any references for this?
>
>It is part of the regulator subsystem and the standard framework there to
>handle regulators with an enumerable number of specific voltage levels.
>
>> IMHO, we do not need to overengineered it to add custom selector
>> regulator just to suit this Keem Bay HW design.
>
>That can be said about a lot of things we model with vqmmc.
>Using standard abstractions makes things easier for maintainers.
>We mostly design abstractions for maintenance not for the simplest way to
>set bits in registers.

Thanks for your input. I will try model it similar to current regulator implementation
specific for keem bay SoC where this keem bay regulator will encapsulated the 
Secure Monitor Calling Convention (SMCCC) during voltage operations.

>
>Yours,
>Linus Walleij

Patch
diff mbox series

diff --git a/drivers/mmc/host/sdhci-of-arasan.c b/drivers/mmc/host/sdhci-of-arasan.c
index 196e3d65277e..00ecdf54128c 100644
--- a/drivers/mmc/host/sdhci-of-arasan.c
+++ b/drivers/mmc/host/sdhci-of-arasan.c
@@ -23,6 +23,7 @@ 
 #include <linux/regmap.h>
 #include <linux/of.h>
 #include <linux/firmware/xlnx-zynqmp.h>
+#include <linux/firmware/intel/keembay.h>
 
 #include "cqhci.h"
 #include "sdhci-pltfm.h"
@@ -79,6 +80,8 @@  struct sdhci_arasan_soc_ctl_field {
  * @baseclkfreq:	Where to find corecfg_baseclkfreq
  * @clockmultiplier:	Where to find corecfg_clockmultiplier
  * @support64b:		Where to find SUPPORT64B bit
+ * @otap_delay:		Where to find otap_delay
+ * @sel_clk_buffer:	Where to find clock buffer delay
  * @hiword_update:	If true, use HIWORD_UPDATE to access the syscon
  *
  * It's up to the licensee of the Arsan IP block to make these available
@@ -89,6 +92,8 @@  struct sdhci_arasan_soc_ctl_map {
 	struct sdhci_arasan_soc_ctl_field	baseclkfreq;
 	struct sdhci_arasan_soc_ctl_field	clockmultiplier;
 	struct sdhci_arasan_soc_ctl_field	support64b;
+	struct sdhci_arasan_soc_ctl_field	otap_delay;
+	struct sdhci_arasan_soc_ctl_field	sel_clk_buffer;
 	bool					hiword_update;
 };
 
@@ -189,6 +194,8 @@  static const struct sdhci_arasan_soc_ctl_map intel_keembay_soc_ctl_map = {
 	.baseclkfreq = { .reg = 0x0, .width = 8, .shift = 14 },
 	.clockmultiplier = { .reg = 0x4, .width = 8, .shift = 14 },
 	.support64b = { .reg = 0x4, .width = 1, .shift = 24 },
+	.otap_delay = { .reg = 0x24, .width = 5, .shift = 23 },
+	.sel_clk_buffer = { .reg = 0x2c, .width = 3, .shift = 25 },
 	.hiword_update = false,
 };
 
@@ -364,6 +371,144 @@  static int sdhci_arasan_voltage_switch(struct mmc_host *mmc,
 	return -EINVAL;
 }
 
+/**
+ * sdhci_arasan_keembay_io_line_supply_operation - Supply for the bus IO line power
+ *
+ * @mmc:	Pointer to sdhci_host
+ * @ios:	Pointer to IO bus setting
+ *
+ * For Keem Bay HW, in order to operate the GPIOs line for Clk,Cmd and Data,
+ * it is important to configure the supplied voltage applied to their I/O Rail
+ * and the output of the i2c expander Pin.
+ *
+ * Note that to configure the voltage rail setting, specific bits in AON_CFG
+ * register must be set. While to configure the i2c expander pin output,
+ * gpio regulator modelling is been used to control the pin state.
+ *
+ * Always on Domain register having different base address from GPIO base address
+ * and it is a secure register. There is no way to control the I/O Voltage Rail
+ * from the GPIO Pad. SMC CALL is been used to set the bits in AON_CFG1 register.
+ *
+ * Final Voltage applied on the GPIOs Line are dependent by both supplied voltage
+ * I/O Rail and expander pin output as it is been set after passing through the
+ * voltage sense resistor.
+ *
+ * Return: 0 on success and error value on error
+ */
+static int sdhci_arasan_keembay_io_line_supply_operation(struct mmc_host *mmc,
+						  struct mmc_ios *ios)
+{
+	int ret, min_uV, max_uV, volt_rail;
+
+	if (ios->signal_voltage == MMC_SIGNAL_VOLTAGE_180) {
+		min_uV = IOV_1V8;
+		max_uV = IOV_1V8;
+		volt_rail = KEEMBAY_SET_1V8_IO_RAIL;
+	} else {
+		min_uV = IOV_3V3;
+		max_uV = IOV_3V3;
+		volt_rail = KEEMBAY_SET_3V3_IO_RAIL;
+	}
+
+	/* If no vqmmc supply then we can't change the voltage */
+	if (IS_ERR(mmc->supply.vqmmc))
+		return -EINVAL;
+
+	ret = regulator_set_voltage(mmc->supply.vqmmc, min_uV, max_uV);
+	if (ret)
+		return ret;
+
+	/*
+	 * This is like a final gatekeeper. Need to ensure changed voltage
+	 * is settled before and after turn on this bit.
+	 */
+	usleep_range(1000, 1100);
+
+	ret = keembay_io_rail_supplied_voltage(volt_rail);
+	if (ret)
+		return ret;
+
+	usleep_range(1000, 1100);
+
+	return 0;
+}
+
+static int sdhci_arasan_keembay_voltage_switch(struct mmc_host *mmc,
+				       struct mmc_ios *ios)
+{
+	struct sdhci_host *host = mmc_priv(mmc);
+	u16 ctrl_2, clk;
+	int ret;
+
+	switch (ios->signal_voltage) {
+	case MMC_SIGNAL_VOLTAGE_180:
+		clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL);
+		clk &= ~SDHCI_CLOCK_CARD_EN;
+		sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
+
+		clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL);
+		if (clk & SDHCI_CLOCK_CARD_EN)
+			return -EAGAIN;
+
+		sdhci_writeb(host, SDHCI_POWER_ON | SDHCI_POWER_180,
+				   SDHCI_POWER_CONTROL);
+
+		ret = sdhci_arasan_keembay_io_line_supply_operation(mmc, ios);
+		if (ret)
+			return ret;
+
+		ctrl_2 = sdhci_readw(host, SDHCI_HOST_CONTROL2);
+		ctrl_2 |= SDHCI_CTRL_VDD_180;
+		sdhci_writew(host, ctrl_2, SDHCI_HOST_CONTROL2);
+
+		/* Sleep for 5ms to stabilize 1.8V regulator */
+		usleep_range(5000, 5500);
+
+		/* 1.8V regulator output should be stable within 5 ms */
+		ctrl_2 = sdhci_readw(host, SDHCI_HOST_CONTROL2);
+		if (!(ctrl_2 & SDHCI_CTRL_VDD_180))
+			return -EAGAIN;
+
+		clk  = sdhci_readw(host, SDHCI_CLOCK_CONTROL);
+		clk |= SDHCI_CLOCK_CARD_EN;
+		sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
+		break;
+	case MMC_SIGNAL_VOLTAGE_330:
+		ret = sdhci_arasan_keembay_io_line_supply_operation(mmc, ios);
+		if (ret)
+			return ret;
+
+		/* Set 1.8V Signal Enable in the Host Control2 register to 0 */
+		ctrl_2 = sdhci_readw(host, SDHCI_HOST_CONTROL2);
+		ctrl_2 &= ~SDHCI_CTRL_VDD_180;
+		sdhci_writew(host, ctrl_2, SDHCI_HOST_CONTROL2);
+
+		/* Sleep for 5ms to stabilize 3.3V regulator */
+		usleep_range(5000, 5500);
+
+		/* 3.3V regulator output should be stable within 5 ms */
+		ctrl_2 = sdhci_readw(host, SDHCI_HOST_CONTROL2);
+		if (ctrl_2 & SDHCI_CTRL_VDD_180)
+			return -EAGAIN;
+
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int sdhci_arasan_keembay_select_drive_strength(struct mmc_card *card,
+					unsigned int max_dtr, int host_drv,
+					int card_drv, int *drv_type)
+{
+	if (card->host->ios.signal_voltage == MMC_SIGNAL_VOLTAGE_180)
+		*drv_type = MMC_SET_DRIVER_TYPE_C;
+
+	return 0;
+}
+
 static const struct sdhci_ops sdhci_arasan_ops = {
 	.set_clock = sdhci_arasan_set_clock,
 	.get_max_clock = sdhci_pltfm_clk_get_max_clock,
@@ -964,6 +1109,77 @@  static void sdhci_arasan_update_baseclkfreq(struct sdhci_host *host)
 	sdhci_arasan_syscon_write(host, &soc_ctl_map->baseclkfreq, mhz);
 }
 
+/**
+ * sdhci_arasan_update_otap_delay - Set otap delay
+ *
+ * This is used to manualy control the txclk Tap Delay,
+ * for flopping the final stage flops.
+ *
+ * NOTES:
+ * - Many existing devices don't seem to do this and work fine.  To keep
+ *   compatibility for old hardware where the device tree doesn't provide a
+ *   register map, this function is a noop if a soc_ctl_map hasn't been provided
+ *   for this platform.
+ *
+ * @host:		The sdhci_host
+ */
+static void sdhci_arasan_update_otap_delay(struct sdhci_host *host,
+						u32 value)
+{
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct sdhci_arasan_data *sdhci_arasan = sdhci_pltfm_priv(pltfm_host);
+	const struct sdhci_arasan_soc_ctl_map *soc_ctl_map =
+		sdhci_arasan->soc_ctl_map;
+
+	/* Having a map is optional */
+	if (!soc_ctl_map)
+		return;
+
+	/* If we have a map, we expect to have a syscon */
+	if (!sdhci_arasan->soc_ctl_base) {
+		pr_warn("%s: Have regmap, but no soc-ctl-syscon\n",
+			mmc_hostname(host->mmc));
+		return;
+	}
+
+	sdhci_arasan_syscon_write(host, &soc_ctl_map->otap_delay, value);
+}
+
+/**
+ * sdhci_arasan_update_clk_sel_buff - Clock buffer select
+ *
+ * This is used to delay the clock buffer
+ *
+ * NOTES:
+ * - Many existing devices don't seem to do this and work fine.  To keep
+ *   compatibility for old hardware where the device tree doesn't provide a
+ *   register map, this function is a noop if a soc_ctl_map hasn't been provided
+ *   for this platform.
+ *
+ * @host:		The sdhci_host
+ */
+static void sdhci_arasan_update_sel_clkbuf(struct sdhci_host *host,
+						u32 value)
+{
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct sdhci_arasan_data *sdhci_arasan = sdhci_pltfm_priv(pltfm_host);
+	const struct sdhci_arasan_soc_ctl_map *soc_ctl_map =
+		sdhci_arasan->soc_ctl_map;
+
+	/* Having a map is optional */
+	if (!soc_ctl_map)
+		return;
+
+	/* If we have a map, we expect to have a syscon */
+	if (!sdhci_arasan->soc_ctl_base) {
+		pr_warn("%s: Have regmap, but no soc-ctl-syscon\n",
+			mmc_hostname(host->mmc));
+		return;
+	}
+
+	sdhci_arasan_syscon_write(host, &soc_ctl_map->sel_clk_buffer, value);
+}
+
 static void sdhci_arasan_set_clk_delays(struct sdhci_host *host)
 {
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
@@ -1587,6 +1803,34 @@  static int sdhci_arasan_probe(struct platform_device *pdev)
 		host->mmc->caps |= MMC_CAP_WAIT_WHILE_BUSY;
 	}
 
+	if (of_device_is_compatible(np, "intel,keembay-sdhci-5.1-sd")) {
+		struct device_node *phys;
+		u32 otap_delay, sel_clk_buffer;
+
+		phys = of_parse_phandle(dev->of_node, "phys", 0);
+		if (!phys) {
+			dev_err(dev, "Can't get phys for sd0\n");
+			ret = -ENODEV;
+			goto err_pltfm_free;
+		}
+
+		of_property_read_u32(phys,
+			"intel,keembay-emmc-phy-otap-dly", &otap_delay);
+		of_property_read_u32(phys,
+			"intel,keembay-emmc-phy-sel-clkbuf", &sel_clk_buffer);
+
+		of_node_put(phys);
+
+		sdhci_arasan_update_otap_delay(host, otap_delay);
+		sdhci_arasan_update_sel_clkbuf(host, sel_clk_buffer);
+
+		host->mmc_host_ops.start_signal_voltage_switch =
+			sdhci_arasan_keembay_voltage_switch;
+
+		host->mmc_host_ops.select_drive_strength =
+			sdhci_arasan_keembay_select_drive_strength;
+	}
+
 	sdhci_arasan_update_baseclkfreq(host);
 
 	ret = sdhci_arasan_register_sdclk(sdhci_arasan, clk_xin, dev);